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

Fix setting token for security schemes other than basic and bearer #252

Merged
merged 1 commit into from
May 22, 2024

Conversation

zlondrej
Copy link
Contributor

The use-case I have for this is combination of session cookie and CSRF protection header.

While the cookie is handled automatically by browser, the token can be set manually in the Authentication section. However it can't be set using setAuthenticationConfiguration method, because the conditions only allow certain schemes to be configured.

image

Rather than preventing valid cases, the method should allow configuring any security scheme, even if it doesn't necessarily make sense.

@zlondrej zlondrej changed the base branch from release/2.2 to main May 22, 2024 17:05
@zlondrej zlondrej changed the base branch from main to release/2.2 May 22, 2024 17:06
@wparad
Copy link
Member

wparad commented May 22, 2024

Agreed, although there is no reason that a user would ever need to have a X-CSRF-Token to actually put in here, and even if they did this is almost certainly the wrong way to set it. There is no reason why we would assume that the Auth header value must be either Basic or Bearer. So it makes sense to pull in this change.

Thank you.

@wparad wparad changed the title Allow setting keys for any security scheme Fix setting token for security schemes other than basic and bearer May 22, 2024
@wparad wparad merged commit 8113a40 into Authress-Engineering:release/2.2 May 22, 2024
2 checks passed
@wparad
Copy link
Member

wparad commented May 22, 2024

Published in version 2.2.710

@zlondrej
Copy link
Contributor Author

Well, that was way faster than I expected, I didn't even manage to write the changelog. 😬

@wparad
Copy link
Member

wparad commented May 22, 2024

Well, that was way faster than I expected, I didn't even manage to write the changelog. 😬

Honestly I thought about it, but I don't think anyone has been waiting for this feature. Most people don't expose the token input, so it isn't he highest used feature OR they are using Bearer/Basic input. So I didn't think we needed to take that extra step here.

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.

2 participants