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

chore: ⚰️ remove authelia 4.37 and below comments #554

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

nemchik
Copy link
Member

@nemchik nemchik commented Mar 25, 2025

linuxserver.io


  • I have read the contributing guideline and understand that I have made the correct modifications

Description:

Benefits of this PR and context:

How Has This Been Tested?

Source / References:

@nemchik nemchik requested a review from a team March 25, 2025 19:50
@LinuxServer-CI
Copy link
Contributor

I am a bot, here is the pushed image/manifest for this PR:

ghcr.io/linuxserver/lspipepr-swag:3.3.0-pkg-afdf7ef9-dev-15a3bc9d2c8c6a6c6dc2e809cf4c2318953341f8-pr-554

@nemchik
Copy link
Member Author

nemchik commented Mar 25, 2025

Added new commit to set the proxy_path to include the full subpath starting with /api so that authelia doesn't need to be configured to use the /authelia subpath. Also removed any need for the /authelia subpath in the authelia confs (simplifying things).

Ref: https://www.authelia.com/integration/proxies/swag/#adjust-authelia-serverconf

@LinuxServer-CI
Copy link
Contributor

I am a bot, here is the pushed image/manifest for this PR:

ghcr.io/linuxserver/lspipepr-swag:3.3.0-pkg-afdf7ef9-dev-563ae7e9c5aec250ef21bd25afaf29b4b414fbd3-pr-554

@LinuxServer-CI
Copy link
Contributor

I am a bot, here is the pushed image/manifest for this PR:

ghcr.io/linuxserver/lspipepr-swag:3.3.0-pkg-fcfbefb7-dev-351f8a9bc065eab185d437cb4a9c4be8cd9a5d57-pr-554

@nemchik
Copy link
Member Author

nemchik commented Apr 5, 2025

@drizuid Is this good to go?

@LinuxServer-CI
Copy link
Contributor

I am a bot, here is the pushed image/manifest for this PR:

ghcr.io/linuxserver/lspipepr-swag:3.3.0-pkg-fcfbefb7-dev-8b8d33a81a84c6fe80c13514c1918a9bee1c393b-pr-554

@drizuid
Copy link
Member

drizuid commented Apr 6, 2025

@drizuid Is this good to go?

Im on vacation and dont use our confs, it might be better to have someone that encountered an issue test or have those authelia guys (that we hope will provide support for the confs as-is) test; but i will give a visual on monday

@@ -62,11 +33,6 @@ location @authelia_proxy_signin {
## Translate the Location response header from the auth subrequest into a variable
auth_request_set $signin_url $upstream_http_location;

Copy link
Member

Choose a reason for hiding this comment

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

i think you need the below, at least I have this in mine

    if ($signin_url = '') {
        ## Set the $signin_url variable
        set $signin_url https://$upstream_authelia/authelia/auth-request;
    }

https://www.authelia.com/reference/guides/proxy-authorization/#authrequest
and
https://www.authelia.com/blog/4.38-release-notes/
For example if your previous proxy configuration included http://authelia:9091/api/verify?rd=https://auth.example.com it now by default becomes http://authelia:9091/api/authz/forward-auth for Traefik / Caddy / HAProxy, by default becomes http://authelia:9091/api/authz/auth-request for NGINX based proxies, and by default becomes http://authelia:9091/api/authz/ext-authz for Envoy.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was removed because signin_url should always be set via header response from authelia in 4.38. If it somehow does not get set that would mean authelia didn't send it and the auth has failed in some way.

Having set $signin_url https://$upstream_authelia/authelia/auth-request; would need authelia configured to respond with the subpath /authelia, which we're removing comment instructions to configure since that seemed to be a complication with users trying to set up authelia.

Copy link
Member

@drizuid drizuid Apr 6, 2025

Choose a reason for hiding this comment

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

I do not have the subpath set and what I showed works fine (believe it listens on all paths, so likely I could remove the Authelia path and it'd still work, admittedly I made my config before the migration guide came out from them), we will definitely need to test because I'm pretty sure the 4.38 migration guide says it's needed(without the subpath )(I think I linked but I'm on mobile)

@nemchik
Copy link
Member Author

nemchik commented Apr 6, 2025

@drizuid Is this good to go?

Im on vacation and dont use our confs, it might be better to have someone that encountered an issue test or have those authelia guys (that we hope will provide support for the confs as-is) test; but i will give a visual on monday

I'm in agreement that asking someone from the Authelia discord server is a good move. I have not tested these confs either. I'm currently not running any auth 😅

I do recall one user had made the adjustments I recommended but it was more ad-hoc and not that they had the fully modified files from this PR. I'll ping in their discord to ask if someone wouldn't mind testing and then replying here to let us know.

Copy link
Member

@drizuid drizuid left a comment

Choose a reason for hiding this comment

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

based on testing from hendrik and my visual inspection, i approve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: PRs Approved
Development

Successfully merging this pull request may close these issues.

3 participants