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

Auth0EndSessionEndpoint mangles URLs #221

Closed
ronhitchens opened this issue Apr 20, 2020 · 4 comments
Closed

Auth0EndSessionEndpoint mangles URLs #221

ronhitchens opened this issue Apr 20, 2020 · 4 comments
Assignees
Labels
type: bug Something isn't working
Milestone

Comments

@ronhitchens
Copy link

I observed that the URL location returned from the Auth0 EndSession handler was malformed, which caused OAuth logout operations to not work properly. The URL I saw in the redirect response was:

https:/mydomain.com/v2/logout

Note the single slash after https.

After much digging into the Micronaut code, I determined that the URL being generated in Auth0EndSessionEndpoint.getUrl() was the source of the problem. The configured Auth0 issuer URL was correct: https://mydomain.com.

A quick test showed that StringUtils.prependUri() was the culprit (in Groovy):

static void main (String[] args) {
	println StringUtils.prependUri ('https://mydomain.com', '/v2/logout')
}

Yields:

https:/mydomain.com/v2/logout

Of course it should be:

https://mydomain.com/v2/logout

That's some broken code right there.

My IDE tells me that there are eight places where StringUtils.prependUri is called, including DefaultHttpClient, which is a bit worrying.

Environment Information

  • Operating System: MacOs / AWS Linux
  • Micronaut Version: 1.3.4
  • JDK Version: 1.8
@graemerocher
Copy link
Contributor

The method is designed to work on paths not full URIs. Looking for usages the only place it seems to be being incorrectly used is prependContextPath in DefaultHttpClient and only when a context path is set. We need to review this usage as that one usage may indeed be a bug.

@ronhitchens
Copy link
Author

Well, that is certainly not correct.

I did not mis-use the method, this is a bug entirely in the Micronaut code. I only found the erroneous output when debugging why my OAuth/Auth0 integration was not working as expected for logout.

As described in your own OAuth Security Guide docs (https://micronaut-projects.github.io/micronaut-security/latest/guide/#oauth) I set the issuer property to https://mydomain.auth0.com.

The description in Table 1 of Section 11.3.1.2.1 says:

URL using the https scheme with no query or fragment component that the Open ID provider asserts as its issuer identifier.

I'm using the Auth0 OAuth configuration with an EndSession handler. That results in the class Auth0EndSessionEndpoint being loaded.

This is the method getUrl in that class:

    protected String getUrl() {
        URL url = clientConfiguration.getOpenid()
                .flatMap(OpenIdClientConfiguration::getIssuer)
                .get();
        return StringUtils.prependUri(url.toString(), "/v2/logout");
    }

The value of url comes from the property micronaut.security.oauth.clients.auth0.openid.issuer, which is set to the aforementioned https://mydomain.auth0.com.

As described in my original post, this results in StringUtils.prependUri ("https://mydomain.auth0.com", "/v2/logout") being called, which returns the erroneous https:/mydomain.auth0.com/v2/logout.

So, long story short, this is a serious bug in your code, the Auth0 EndSession handler is broken. Please fix it. I also suggest you take a closer look at all the other places this method is called and make sure they're not passing parameters that result in bad URLs as well.

Thanks you.

@jameskleeh jameskleeh transferred this issue from micronaut-projects/micronaut-core Apr 22, 2020
@jameskleeh jameskleeh changed the title io.micronaut.core.util.StringUtils.prependUri() mangles URLs Auth0EndSessionEndpoint mangles URLs Apr 22, 2020
@graemerocher
Copy link
Contributor

I never implied you misused the API it is indeed a bug and thanks for the report. I would encourage you to communicate with Open Source maintainers in a less passive aggressive tone however. Also PRs welcome.

@ronhitchens
Copy link
Author

Hmm. When I clearly describe a problem, with an example, and point at the exact place where the bug exists, and you reply with:

The method is designed to work on paths not full URIs. Looking for usages the only place it seems to be being incorrectly used is prependContextPath in DefaultHttpClient and only when a context path is set.

That sure seems like you're telling me I'm wrong and there is no bug. If I misunderstood what you meant to say, I apologize for that. I would encourage you to communicate with Bug Reporters in less ambiguous language.

Thank you.

@jameskleeh jameskleeh added this to the 1.3.2 milestone Apr 27, 2020
@jameskleeh jameskleeh self-assigned this Apr 27, 2020
@jameskleeh jameskleeh added the type: bug Something isn't working label Apr 27, 2020
@jameskleeh jameskleeh modified the milestones: 1.3.2, 1.4.1 Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants