Added support for '{realm}' placeholder in authorizationUrl and tokenUrl

[Download]

This PR addresses an issue I was having with Keycloak, which adds the realm to the path part of the URL, forcing me to hardcode the realm in the definition.

After this PR is merged, people will be able to use a special placeholder token, :realm in the URL and it will be replaced with the realm specified in the call to initOAuth:

{
	"authorizationUrl": "http://localhost:8080/auth/realms/:realm/protocol/openid-connect/auth",
	"tokenUrl": "http://localhost:8080/auth/realms/:realm/protocol/openid-connect/token",
}

Note the use of the :realm placeholder in the URLs.

This is what I did:

• Factored out URL handling code into function processUrl
• Call processUrl in places where we use authorizationUrl and tokenUrl
• Removed tiny bit of redundant code in auths.jsx
• Set default/example realm name to 'your-realm' (singular) instead of 'your-realms' (plural) as this parameter is supposed to be singular afaik
• Improved documentation for 'realm' parameter in README.md

I ran the tests. There were no failures but some tests are reported as 'pending'. I am not sure it is normal but I cannot really imagine my changes would have caused this so I ignored it.

106 passing (646ms)
  11 pending

This is my first PR for swagger-ui so I think it would be best if you took a moment (extra) to closely inspect my changes. Let me know if you see any issues and I'll try to address them ASAP.

Fixes #3406 See also #1424

EDIT: Oh, in case you wondered why I would choose :realm as the token... It's a 'standard' used in Express JS and indeed any server / router built with path-to-regexp. So it (should) feel at home with anyone who has used such systems in the past.

EDIT 2: I realized that the system would be much more flexible if processUrl does not rely on the URLs having no query parameters to begin with. So I added a commit that tests whether the URL already contains a question mark. If it does it uses ampersand to append the query parameters. This should enable URLs like this:

{
	"authorizationUrl": "http://localhost:8080/openid/auth/?keycloak_realm=:realm",
	"tokenUrl": "http://localhost:8080/openid/token?keycloak_realm=:realm",
}

Note the use of the :realm placeholder in the URLs, assigned to custom querystring parameters.

As far as I can tell the 'realm' parameter is not standardized in OAuth and so providers are free to choose their own mechanism (as witnessed by Keycloak's choice for path parameters). So it's probably best to be as flexible as possible here.

[webron]

@Download - Thanks for taking the time to submit the PR.

I'm inclined to not accept the PR as-is for a few reasons:

1. The use case seems non-standard, and I don't want to introduce a solution that solves a problem to a small set of users. I understand however that a solution already exists right now and you're expanding on it - so I'd appreciate some relevant resources you might be able to refer me to, to read on that.
2. While the standard in javascript-based framework is to use something like :realm to denote a variable, I'd rather stick to the standard used by the spec, meaning it would end up being {realm}.

Once I understand the topic better with your references, I'll ask @shockey to review the code.

[Download]

Hi Ron,

Can you let me know what I would have to change to get this merged?

While the standard in javascript-based framework is to use something like :realm to denote a variable, I'd rather stick to the standard used by the spec, meaning it would end up being {realm}.

Ok, that would be a small and simple change.

The use case seems non-standard

It seems as if that's by design? As in, the realm parameter seems to be a left-over from OAuth1 that is no longer in the spec, but is in many provider URLs? I'm basing that on this issue from this very project:

Neither the realm nor resource parameters are part of the OAuth spec. so not really a bug, but not useful either. I can confirm that changing realm to resource when constructing the token URL in swagger-oauth.js allows Swagger UI to work with the AAD OAuth flow (though not in IE).

I'd appreciate some relevant resources you might be able to refer me to, to read on that.

Well I am using Keycloak. It has some docs about Realm. It's a pretty big/popular system and it's encoding the realm parameter in the path. Check out their REST API docs, it has the realm parameter all over it. And I see now they use the same convention as you proposed, with curly braces: {realm}.

Apart from the rename of the placeholder I'm not sure what I would have to change to get this in?

EDIT: I updated the PR with the new placeholder name {realm}.

[jirikuncar]

Thanks @Download ! I am having similar problem with our KeyCloak and Swagger-UI. I hope this PR makes it to upstream.

[Download]

@webron Can you maybe re-evaluate?

[webron]

@Download I hope to get back to it shortly, a bit overloaded at the moment. Thanks for the reminder and your patience.

[Download]

Ugh I did not realize all that conversation would end up here! I guess I should have picked 'Start a Review'. Sorry!

Anyway, @webron I added some comments to explain what I did and why. Hopefully this can help you a bit when you get the time to review this.

[shockey]

@webron, gentle bump 🌞

[Download]

@webron Gentle bump 👍

Other people also are missing this feature. See #3406

[shockey]

On hold pending @webron :phone:

[Mathias02]

This appears to be a branch merge conflict that falls under the responsibility of maintainers if I am not mistaken,