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

Rework Authentication into separate services #1491

Merged
merged 5 commits into from
Nov 24, 2024
Merged

Rework Authentication into separate services #1491

merged 5 commits into from
Nov 24, 2024

Conversation

jtojnar
Copy link
Member

@jtojnar jtojnar commented Jul 10, 2024

This is a from-scratch rewrite, moving a bit closer to Single Responsibility Principle.

We split handling of credentials-in-config and always-open authentication systems.
In the future, we will be able implement more methods this way.

This was motivated by session code being called in constructor,
which would break in CLI with Tracy strict mode.

For now, we are just porting the Authentication helper and controller.

Additionally:

  • Session verification now also checks if the credentials in the config did not change.
  • Requests from loopback IP address now give full access to all operations, not just update.
  • IPv6 loopback address is recognized as well.
  • Requests forwarded by proxies are filtered out since local reverse proxies might come from loopback as well.

One thing I do not like that any request with credentials will automatically
persist the login to session but removing that feature can be done later.

Copy link

netlify bot commented Jul 10, 2024

Deploy Preview for selfoss canceled.

Name Link
🔨 Latest commit cf74581
🔍 Latest deploy log https://app.netlify.com/sites/selfoss/deploys/67433f5ea2c3e30008152c8b

@jtojnar jtojnar force-pushed the auth branch 3 times, most recently from 58d8664 to a36d706 Compare July 11, 2024 01:42
So that it does not try to access `$_SERVER['SERVER_NAME']` in CLI SAPI.

Though this is not sufficient, since the Authentication helper will try to access it strictly too.
So that we do not need to care about it in the Authentication rewrite later.
Otherwise unexpected success will not be caught.
Rename authorization levels to `read`/`update`/`privileged` and explicitly document them.
This is a from-scratch rewrite, moving a bit closer to Single Responsibility Principle.

We split handling of credentials-in-config and always-open authentication systems.
In the future, we will be able implement more methods this way.

This was motivated by session code being called in constructor,
which would break in CLI with Tracy strict mode.

For now, we are just porting the Authentication helper and controller.

Additionally:

- Session verification now also checks if the credentials in the config did not change.
- Requests from loopback IP address now give full access to all operations, not just update.
- IPv6 loopback address is recognized as well.
- Requests forwarded by proxies are filtered out since local reverse proxies might come from loopback as well.

One thing I do not like that any request with credentials will automatically
persist the login to session but removing that feature can be done later.
@jtojnar jtojnar marked this pull request as ready for review November 24, 2024 15:05
@jtojnar jtojnar merged commit cf74581 into master Nov 24, 2024
15 checks passed
@jtojnar jtojnar deleted the auth branch November 24, 2024 15:05
@jtojnar jtojnar added this to the 2.20 milestone Nov 26, 2024
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.

1 participant