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

Download · 2017-07-18T21:19:31Z

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.

* 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' in README.md Fixes swagger-api#3406 See also swagger-api#1424

In processUrl, at the end, it adds the query parameters to the URL, but it should first test whether the URL already contains a question mark. Only add a question mark to the URL when it did not already have one, otherwise use ampersand.

webron · 2017-07-20T23:35:30Z

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

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

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.
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 · 2017-07-21T11:16:46Z

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}.

Changed `'your-realms'` to `'your-realm'` in example in docs as this param has to be singular AFAIK

jirikuncar · 2017-07-27T12:57:57Z

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

Download · 2017-07-31T08:23:50Z

@webron Can you maybe re-evaluate?

webron · 2017-07-31T18:03:12Z

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

Download · 2017-08-01T10:29:27Z

src/core/components/auth/auths.jsx

@@ -109,15 +109,14 @@ export default class Auths extends React.Component {
            <p>API requires the following scopes. Select which ones you want to grant to Swagger UI.</p>
          </div>
            {
-              definitions.filter( schema => schema.get("type") === "oauth2")
-                .map( (schema, name) =>{
+              oauthDefinitions.map( (schema, name) =>{


oauthDefinitions is defined above as definitions.filter( schema => schema.get("type") === "oauth2") so repeating that code here was redundant. I stumbled upon this during my research so I fixed it... Should maybe have been a separate PR.

Download · 2017-08-01T10:31:19Z

README.md

@@ -115,7 +115,7 @@ const ui = SwaggerUIBundle({...})
 ui.initOAuth({
    clientId: "your-client-id",
    clientSecret: "your-client-secret-if-required",
-    realm: "your-realms",
+    realm: "your-realm",


Afaict from all the info I've read on realm, it's usually like a container around users, roles, etc. So it seems strange to be targeting multiple realms. It for sure won't work with Keycloak. So this is why I think it is better if the example is singular.

Download · 2017-08-01T10:33:30Z

README.md

@@ -103,7 +103,7 @@ Config Name | Description
 --- | ---
 client_id | Default clientId. MUST be a string
 client_secret | Default clientSecret. MUST be a string
-realm | realm query parameter (for oauth1) added to `authorizationUrl` and `tokenUrl` . MUST be a string
+realm | The OAuth realm parameter for `authorizationUrl` and `tokenUrl`. Optional. If specified it MUST be a string. If a `{realm}` placeholder is present in the URL path, it will be replaced with this value. If no such placeholder is present, the value will be added as the value for querystring parameter 'realm'.


The old docs were very concise. I added some more detail and mentioned the {realm} placeholder and realm query parameter explicitly so people (hopefully) understand what it does.

Download · 2017-08-01T10:34:51Z

src/core/components/auth/auths.jsx

                  return (<div key={ name }>
                    <Oauth2 authorized={ authorized }
                            schema={ schema }
                            name={ name } />
                  </div>)
                }
-                ).toArray()
+              ).toArray()


Some whitespace conversion by my editor probably, sorry.

Download · 2017-08-01T10:35:39Z

src/core/components/auth/oauth2.jsx

@@ -27,6 +27,7 @@ export default class Oauth2 extends React.Component {
    let authConfigs = authSelectors.getConfigs() || {}
    let username = auth && auth.get("username") || ""
    let clientId = auth && auth.get("clientId") || authConfigs.clientId || ""
+    let realm = auth && auth.get("realm") || authConfigs.realm || ""


Make sure we get the realm setting as well

Download · 2017-08-01T10:36:15Z

src/core/components/auth/oauth2.jsx

@@ -39,7 +40,8 @@ export default class Oauth2 extends React.Component {
      clientSecret: clientSecret,
      username: username,
      password: "",
-      passwordType: passwordType
+      passwordType: passwordType,
+      realm: realm
    }


Add it to the state so we can access it later

Download · 2017-08-01T10:38:01Z

src/core/components/auth/oauth2.jsx

-        { ( flow === IMPLICIT || flow === ACCESS_CODE ) && <p>Authorization URL: <code>{ schema.get("authorizationUrl") }</code></p> }
-        { ( flow === PASSWORD || flow === ACCESS_CODE || flow === APPLICATION ) && <p>Token URL:<code> { schema.get("tokenUrl") }</code></p> }
+        { ( flow === IMPLICIT || flow === ACCESS_CODE ) && <p>Authorization URL: <code>{ processUrl(schema.get("authorizationUrl"), this.state) }</code></p> }
+        { ( flow === PASSWORD || flow === ACCESS_CODE || flow === APPLICATION ) && <p>Token URL:<code> { processUrl(schema.get("tokenUrl"), this.state) }</code></p> }


Call processUrl() on the authorizationUrl and tokenUrl so we correctly fill in the {realm} and other parameters.

Download · 2017-08-01T10:39:24Z

src/core/oauth2-authorize.js

-  }
-
-  let url = [schema.get("authorizationUrl"), query.join("&")].join("?")
+  const url = processUrl(schema.get("authorizationUrl"), authConfigs, query)


The removed code above is now part of processUrl

Download · 2017-08-01T10:41:17Z

src/core/oauth2-authorize.js

+    }
+  }
+  return result
+}


This is the meat of it. The existing code to fill in the configs and additional query params is now in this function, where I also added the bit that does the replacement of the placeholder.

Download · 2017-08-01T10:42:18Z

src/core/plugins/auth/actions.js

-
-  for (let key in additionalQueryStringParams) {
-    url += "&" + key + "=" + encodeURIComponent(additionalQueryStringParams[key])
-  }


This is now done by processUrl

Download · 2017-08-01T10:46:23Z

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 · 2017-09-29T21:52:45Z

@webron, gentle bump 🌞

Download · 2018-02-02T20:52:36Z

@webron Gentle bump 👍

Other people also are missing this feature. See #3406

shockey · 2018-03-03T01:01:24Z

On hold pending @webron ☎️

Download added 4 commits July 18, 2017 23:08

Merge branch 'master' into support_realm_in_path

3525003

Only add question mark when not there yet.

79c7ab7

In processUrl, at the end, it adds the query parameters to the URL, but it should first test whether the URL already contains a question mark. Only add a question mark to the URL when it did not already have one, otherwise use ampersand.

Merge branch 'master' into support_realm_in_path

f1a710d

Download added 2 commits July 21, 2017 13:16

Merge branch 'master' into support_realm_in_path

2fc3b50

Changed :realm placeholder to {realm}

bb94db0

Changed `'your-realms'` to `'your-realm'` in example in docs as this param has to be singular AFAIK

Merge branch 'master' into support_realm_in_path

30122e9

Download changed the title ~~Added support for ':realm' placeholder in authorizationUrl and tokenUrl~~ Added support for '{realm}' placeholder in authorizationUrl and tokenUrl Jul 31, 2017

Merge branch 'master' into support_realm_in_path

7fa5543

Download commented Aug 1, 2017

View reviewed changes

Download added 3 commits August 4, 2017 20:10

Merge branch 'master' into support_realm_in_path

701e867

Merge branch 'master' into support_realm_in_path

f71044d

Merge branch 'master' into support_realm_in_path

64a2467

shockey requested a review from webron August 16, 2017 19:04

Merge branch 'master' into support_realm_in_path

898331b

This was referenced Aug 31, 2017

Why don't host and basePath support path templating? OAI/OpenAPI-Specification#169

Closed

Environment Variables in definition OAI/Overlay-Specification#33

Open

Merge branch 'master' into support_realm_in_path

d9ec8ab

Download mentioned this pull request Feb 2, 2018

How to specify OAuth 'realm' #3406

Open

shockey added the On hold label Mar 3, 2018

shockey removed the status: on hold label May 16, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

Download commented Jul 18, 2017 •

edited

Loading

webron commented Jul 20, 2017

Download commented Jul 21, 2017 •

edited

Loading

jirikuncar commented Jul 27, 2017

Download commented Jul 31, 2017

webron commented Jul 31, 2017

Download Aug 1, 2017

Download Aug 1, 2017

Download Aug 1, 2017

Download Aug 1, 2017

Download Aug 1, 2017

Download Aug 1, 2017

Download Aug 1, 2017

Download Aug 1, 2017

Download Aug 1, 2017

Download Aug 1, 2017

Download commented Aug 1, 2017

shockey commented Sep 29, 2017

Download commented Feb 2, 2018

shockey commented Mar 3, 2018

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

Are you sure you want to change the base?

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

Conversation

Download commented Jul 18, 2017 • edited Loading

webron commented Jul 20, 2017

Download commented Jul 21, 2017 • edited Loading

jirikuncar commented Jul 27, 2017

Download commented Jul 31, 2017

webron commented Jul 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Download commented Aug 1, 2017

shockey commented Sep 29, 2017

Download commented Feb 2, 2018

shockey commented Mar 3, 2018

Download commented Jul 18, 2017 •

edited

Loading

Download commented Jul 21, 2017 •

edited

Loading