feat(web): introduction of an access denied page #7251
Conversation
When a user does not have the required permissions, a redirection url to a new 'Access Denied' page is passed back to the reverse proxy along with the 403 error code. This follows the same sequence as an unauthenticated user getting redirected to the login. The Access Denied page presents the user with an option to logout, as well as the details of the page they were denied access to. Work includes a slight refactor of authz handlers to allow for code reuse. Also includes a simple 'demo home' button injected into the development login page to allow for easier navigation back to home.example.com for the Standalone suite. Fixes authelia#2319. Signed-off-by: Rocket Proto <[email protected]>
|
Thanks for choosing to contribute @rocketproto. 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
|
WalkthroughThe recent updates primarily focus on refining the authorization handling across various modules in the system. New methods and types have been introduced to manage forbidden access more effectively, along with improvements in redirection logic and error handling. Additionally, enhancements in the web interface include new error views and better management of query parameters and loading states. Changes
Assessment against linked issues
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
| if authn.Type == AuthnTypeAuthorization { | ||
| ctx.Logger.Infof("Access to %s (method %s) is forbidden for user %s, responding with status code %d", authn.Object.URL.String(), authn.Method, authn.Username, fasthttp.StatusForbidden) | ||
| ctx.ReplyForbidden() | ||
| return |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
return statements should not be cuddled if block has more than two lines (wsl)
|
For quick view: The access denied page: Noting the "Demo Home" link is only injected into the page by the vite config while being served in the demo container. It will not be in the production build. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| @@ -29,13 +29,17 @@ type Authz struct { | |||
|
|
|||
| handleAuthorized HandlerAuthzAuthorized | |||
| handleUnauthorized HandlerAuthzUnauthorized | |||
| handleForbidden HandlerAuthzForbidden | |||
There was a problem hiding this comment.
Add documentation for handleForbidden.
It's important to maintain consistency in code documentation. Please add a comment describing the purpose and usage of the handleForbidden field in the Authz struct.
| // HandlerAuthzForbidden is a Authz handler func that handles forbidden responses. | ||
| type HandlerAuthzForbidden func(ctx *middlewares.AutheliaCtx, authn *Authn, redirectionURL *url.URL) | ||
|
|
There was a problem hiding this comment.
Add documentation for HandlerAuthzForbidden.
The new handler type HandlerAuthzForbidden is missing documentation. Please provide a comment explaining what this handler function is responsible for, similar to the other handler types in this file.
| @@ -45,6 +45,7 @@ const ( | |||
| queryArgConsentID = "consent_id" | |||
| queryArgWorkflow = "workflow" | |||
| queryArgWorkflowID = "workflow_id" | |||
| queryArgEC = "ec" | |||
There was a problem hiding this comment.
Add documentation for queryArgEC.
The new constant queryArgEC is added but lacks documentation. Please add a comment explaining what this constant represents and how it is used within the application.
| baseErrorPath = "error" | ||
| ) | ||
|
|
||
| var ( | ||
| errorForbidden = "forbidden" | ||
| ) |
There was a problem hiding this comment.
Add documentation for baseErrorPath and errorForbidden.
The newly introduced variables baseErrorPath and errorForbidden are missing descriptive comments. Please add documentation to explain their purposes and contexts in which they are used.
| @@ -615,7 +615,7 @@ func (ctx *AutheliaCtx) SpecialRedirectNoBody(uri string, statusCode int) { | |||
| } | |||
|
|
|||
| func (ctx *AutheliaCtx) setSpecialRedirect(uri string, statusCode int) ([]byte, int) { | |||
| if statusCode < fasthttp.StatusMovedPermanently || (statusCode > fasthttp.StatusSeeOther && statusCode != fasthttp.StatusTemporaryRedirect && statusCode != fasthttp.StatusPermanentRedirect && statusCode != fasthttp.StatusUnauthorized) { | |||
| if statusCode < fasthttp.StatusMovedPermanently || (statusCode > fasthttp.StatusSeeOther && statusCode != fasthttp.StatusTemporaryRedirect && statusCode != fasthttp.StatusPermanentRedirect && statusCode != fasthttp.StatusUnauthorized && statusCode != fasthttp.StatusForbidden) { | |||
There was a problem hiding this comment.
Review the status code conditions in setSpecialRedirect.
The condition in the setSpecialRedirect method seems overly complex and might be error-prone. Consider simplifying this condition to improve readability and maintainability.
|
Great work, just a question. If the user who got denied haven't setup TOTP and the denied resource mandates TOTP, will the user then have the ability to request a TOTP token? Because that is the case with the current implementation if you go back to the same URL after the 403 and apply /authelia. I think the current implementation is incorrect. I have a user specifically for one_factor proxies, I don't want that user to have the ability to request a TOTP token if the user in question is not permitted to the resource to begin with. |
When a user does not have the required permissions, a redirection url to a new 'Access Denied' page is passed back to the reverse proxy along with the 403 error code. This follows the same sequence as an unauthenticated user getting redirected to the login. The Access Denied page presents the user with an option to logout, as well as the details of the page they were denied access to.
Work includes a slight refactor of authz handlers to allow for code reuse.
Also includes a simple 'demo home' button injected into the development login page to allow for easier navigation back to home.example.com for the Standalone suite.
Fixes #2319.
Summary by CodeRabbit
New Features
Refactor
Bug Fixes
Documentation
Style