Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: replace HttpConfigurable with adapter to support pre/post-2024.3 (#242) #240

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adietish
Copy link
Contributor

@adietish adietish commented Sep 25, 2024

fixes #242, #255
depends on

@adietish adietish force-pushed the replace_deprecated_proxy_api branch 5 times, most recently from dbc9a20 to b541bbe Compare September 26, 2024 17:42
@adietish adietish self-assigned this Sep 27, 2024
@adietish adietish force-pushed the replace_deprecated_proxy_api branch 2 times, most recently from 1ba34e8 to 46c784e Compare September 27, 2024 09:16
@adietish adietish changed the title fix: replace HttpConfigurable with Adapter to support 2024.3+ fix: replace HttpConfigurable with adapter to support pre/post-2024.3 Sep 27, 2024
@adietish adietish changed the title fix: replace HttpConfigurable with adapter to support pre/post-2024.3 fix: replace HttpConfigurable with adapter to support pre/post-2024.3 (#242) Sep 27, 2024
@adietish adietish force-pushed the replace_deprecated_proxy_api branch 2 times, most recently from 434eeae to 99aadd2 Compare October 4, 2024 12:15
@adietish
Copy link
Contributor Author

adietish commented Oct 4, 2024

putting this on hold, waiting for #246

@adietish
Copy link
Contributor Author

adietish commented Oct 28, 2024

@sbouchet: Tested it in intellij-openshift-connector with IU-2024.1 using our internal proxy. Patch worked, the plugin picked the proxy.
Now waiting for intellij-openshift-connector to compile against IU-2024.2 as in redhat-developer/intellij-openshift-connector#889 to test this patch against EAP of IU-2024.3.

@adietish
Copy link
Contributor Author

adietish commented Nov 1, 2024

@sbouchet tested and corrected the patch to work with latest EAP (243.21565.23). Works for me in 2024.1 and 243.21565.23.
I found out somethid odd though. The existing code (pre-patch) doesn't use proxy credentials that were provided in the settings.

The following (existing) code returns null even if you provided username & password in the settings:

new IdeaWideAuthenticator(HttpConfigurable.getInstance()).getPasswordAuthentication()

The following works on the other hand:

HttpConfigurable.getInstance().getPromptedAuthentication(url, null)

@adietish
Copy link
Contributor Author

adietish commented Nov 1, 2024

Turns out that the reason is as follows:

  1. IdeaWideAuthenticator(HttpConfigurable.getInstance()).getPasswordAuthentication() checks the requestorType using ideaWideAuthenticator.getRequestorType().
  2. The type is null.
  3. IdeaWideAuthenticator.getPasswordAuthentication() therefore ignores the credentials.

https://github.com/JetBrains/intellij-community/blob/f020cd7ad5d49fa5baf4ed526e03d20af690bdb6/platform/platform-api/src/com/intellij/util/net/IdeaWideAuthenticator.java#L32:

public final class IdeaWideAuthenticator extends NonStaticAuthenticator {
  private static final Logger LOG = Logger.getInstance(IdeaWideAuthenticator.class);
  private final HttpConfigurable myHttpConfigurable;

  public IdeaWideAuthenticator(@NotNull HttpConfigurable configurable) {
    myHttpConfigurable = configurable;
  }

  @Override
  public synchronized PasswordAuthentication getPasswordAuthentication() {
    final String host = CommonProxy.getHostNameReliably(getRequestingHost(), getRequestingSite(), getRequestingURL());
    // java.base/java/net/SocksSocketImpl.java:176 : there is SOCKS proxy auth, but without RequestorType passing
    final boolean isProxy = Authenticator.RequestorType.PROXY.equals(getRequestorType()) || "SOCKS authentication".equals(getRequestingPrompt());
    // FIXME prefix for server auth is never used since 7ea74ea400b03cf92d1621ea0e8aa1d386cb886a.
    //  This means that this class manages strictly proxy authentication since 2013

I believe that this is a platform bug. The new API fixes this issue.

@adietish adietish marked this pull request as ready for review November 1, 2024 22:25
@adietish
Copy link
Contributor Author

adietish commented Nov 1, 2024

@sbouchet: please review

ps. current EAP 2024.3 is IC-243.21565.23. It still has the API that's marked as "for removal". So to test if this PR works for it, you'd have to comment out the pre-2024.3 variants in IdeProxyAdapter

gradle.properties:

platformVersion=243.21565.23

As a proxy you can use the internal squid that's available beind VPN. You'd have to set the proxy in the runtime IDE and restart it.

Copy link

sonarcloud bot commented Nov 4, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant