-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
feat(server): custom oidc authorization page for form_post
response method
#4670
base: master
Are you sure you want to change the base?
feat(server): custom oidc authorization page for form_post
response method
#4670
Conversation
Thanks for choosing to contribute @levkoburburas. We lint all PR's with golangci-lint and eslint, I may add a review to your PR with some suggestions. You are free to apply the changes if you're comfortable, alternatively you are welcome to ask a team member for advice. ArtifactsThese changes once approved by a team member will be published for testing on Buildkite, DockerHub and GitHub Container Registry. Docker Container
|
✅ Deploy Preview for authelia-staging ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
form_post
response methodform_post
response method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. However I'd rather this is solved by adjusting the template returned from fosite.FormPostHTMLTemplateProvider
using sha256 hashes instead (in addition to making a special CSP middleware for the authorization endpoint which adds the sha256's when the response mode is form_post
).
Thank you for the review! This could be a great solution, if only this approach also supported passing variables. It is important for me to display the clientID/clientDescription, in fact, where the redirect goes. I will try to make code changes in |
I don't think it's necessary to implement the client description in the short term (nice to have, but not important). However I'm relatively certain they'd be open to implementing an interface addition to the configurator with a signature similar to below: type FormPostResponseProvider interface {
GetWriteAuthorizeFormPostResponse(ctx context.Context) func(redirectURL string, parameters url.Values, template *template.Template, rw io.Writer)
} Or maybe something like this: type FormPostHTMLTemplateExtraValuesProvider interface {
GetFormPostHTMLTemplateExtraValues(ctx context.Context) map[string]interface{}
} If you'd like I can just push the fix and you can work on the nice to have, I'll leave it up to you however. |
I liked your first suggestion, because it seems to me that it gives more opportunities type WriteAuthorizeFormPostResponseProvider interface {
GetWriteAuthorizeFormPostResponse(ctx context.Context) func(rw io.Writer, template *template.Template, redirectURL string, parameters url.Values)
} I created the PR #731 with discussed changes.
Thank you, but it's up to you. I am working with the forked repo, it doesn't bother me. |
By the way, I'm not very good at frontend, perhaps you should check it and maybe improve something at your discretion. |
This pull request solves two issues: