Skip to content

chore: swagger docs omit brower based credentials, rely on swagger auth #13742

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

Merged
merged 2 commits into from
Jul 1, 2024

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Jul 1, 2024

Swagger has an "Authorize" button which should be the only authentication being used in the api requests

Closes #13535

Note: I considered implementing CSRF in the interceptor which would just use the logged in user credentials. But the Authorize button will still exist, and it would be even more confusing since the cookie auth supersedes the header based auth. So swagger requiring explicit authentication feels safer.

Swagger has an "Authorize" button which should be the only
authentication being used in the api requests
@Emyrk Emyrk mentioned this pull request Jul 1, 2024
@Emyrk Emyrk marked this pull request as ready for review July 1, 2024 18:36
@Emyrk Emyrk requested a review from aslilac July 1, 2024 18:36
Copy link
Member

@aslilac aslilac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is awful, but it seems like it's all in ways that aren't your fault, so whatever 😂

//
// So remove authenticating via a cookie, and rely on the authorization
// header passed in.
httpSwagger.UIConfig(map[string]string{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hate that they use a map[string]string for this instead of a struct 😭

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not even a link to the docs 😢

Comment on lines +109 to +112
"requestInterceptor": `(a => {
a.credentials = "omit";
return a;
})`,
Copy link
Member

@aslilac aslilac Jul 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

their recommendation is just to embed javascript in a string??? I extra hate that

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Emyrk Emyrk merged commit 10c2817 into main Jul 1, 2024
36 checks passed
@Emyrk Emyrk deleted the stevenmasley/swagger_docs_error branch July 1, 2024 18:44
@github-actions github-actions bot locked and limited conversation to collaborators Jul 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Swagger CSRF Error
2 participants