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

Respect the name of the api key supplied in spec #1731

Closed
wants to merge 1 commit into from

Conversation

oaktowner
Copy link

The code used to always use "api_key" as the name of the api key query parameter, but the spec has a provision for supplying a different name. This will now use the name of the key supplied in the spec (if one exists), and will fall back to api_key if none is specified.

The code used to always use "api_key" as the name of the api key query parameter, but the spec has a provision for supplying a different name. This will now use the name of the key supplied in the spec (if one exists), and will fall back to api_key if none is specified.
@fehguy
Copy link
Contributor

fehguy commented Nov 6, 2015

Hi @oaktowner, this is a good fix, but let me explain how the security key names work.

Each security definition has a friendly key name. In the default swagger-ui, it's called api_key. In general, it can be anything. For example:

{
  "securityDefinitions": {
    "api_key": {
      "type": "apiKey",
      "name": "api_key",
      "in": "header"
    },
    "petstore_auth": {
      "type": "oauth2",
      "authorizationUrl": "http://petstore.swagger.io/api/oauth/dialog",
      "flow": "implicit",
      "scopes": {
        "write:pets": "modify pets in your account",
        "read:pets": "read your pets"
      }
    }
  }
}

So in this case, there are two security schemes defined: api_key and petstore_auth.

The reason why this matters to this PR is that it assumes you're using api_key as your security name.

We should try to get a general solution, which lets you choose which scheme you're supplying the key for (assuming it's an API key) based on the securityDefinition object.

I do think your PR is better than what's there, but let me see if we can get the general solution applied instead. So I'll keep this open for now :)

@oaktowner
Copy link
Author

One question about that: the method I modified is pretty explicitly called addApiKeyAuthorization, and the problem is that right now it is ignoring what the user supplies as the value for "name" in their API spec.

So I understand that this doesn't address any other situations, but it does seem to fix a bug in the code that exists...right? Or am I missing something?

ikitommi added a commit to metosin/ring-swagger-ui that referenced this pull request Nov 28, 2015
Swagger-ui uses hard-coded `api_key` query-param for api-keys,
with this commit, one can override this in the swagger spec -
both key-name & in (header, query etc.) There are lot's of open
issues related to this, without any resolution time-table. See:

swagger-api/swagger-ui#1766
swagger-api/swagger-ui#1731
swagger-api/swagger-ui#1593
@webron
Copy link
Contributor

webron commented Jun 8, 2017

@oaktowner as we discussed in private, I'm closing this one for now.

@webron webron closed this Jun 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants