Fix authorization headers feature #48
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Having debugged this once via #13 my answer is "I don't know" :) I'd take this or a variation of this change though as it seems that it works for everyone. |
|
To get merged, please rebase/squash it. |
Previous to this update, when I tried to do API-key authorization using headers rather than the query string, Swagger-UI would not send my authorization headers. This update basically makes the authorization options get set AFTER Swagger-UI has finished initializing, which seems to make the system much happier. Changes based on a conversation on a mostly-unrelated pull request, ruby-grape#25
davidbrewer
force-pushed
the
fix-authorization-headers-branch
branch
from
ee4a93e
to
924038c
Compare
|
OK, rebased. I know this change works for my specific use case of a simple api key in an Authorization header (when accompanied with appropriate settings in the initializer). I can't comment on if it will work for more complex use cases like OAuth and so on, but I do think it's probably at the very least a step in the right direction and shouldn't remove any flexibility that was already present. |
dblock
added a commit
that referenced
this pull request
Mar 8, 2016
Fix authorization headers feature
|
Merged. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello! I don't believe this is a ready-to-merge PR, but I thought that this would be the best way to start a discussion of it.
In the discussion of PR #25, @supagroova mentioned a change he had made to the index template which fixed some issues with using authorization headers. I found that a variant on this change was necessary to get my own authorization headers working properly, because otherwise the headers just didn't get sent with requests at all.
The change shown in this PR is similar to what I read on PR #25, except that I kept in the code which gets the api_auth, api_key_name and api_key_type from the grape-swagger-rails configuration. It seems to fix the issue for me, without causing any regressions.
However, I'm unsure how best to write a test that demonstrates that this fixes an issue. I'm also not using this library in an especially complex way so I'm uncertain if I may have broken something that I'm not using myself. Let me know if you have any ideas as to steps I can take to improve this PR to make it something useful for you.