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

[feat] Remote header auth #1731

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

MarcHagen
Copy link
Contributor

Description

  • Allow basic User management without permissions #128 - Not fixes, but adds Remote User functionality through the use off Remote-User (configurable)
  • Ability to skip login form when using Remote-User
  • Validation if the remote user exists in the local database
    This raises the question if we want to auto generate users. But for now I've ignored that part
  • Ability to change username through the cli/command line npm run change-username
  • Hide UI elements when using Disabled Auth or Remote-User headers.

Type of change

Please delete any options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I ran ESLint and other linters for modified files
  • I have performed a self-review of my own code and tested it
  • I have commented my code, particularly in hard-to-understand areas
    (including JSDoc for methods)
  • My changes generate no new warnings
  • My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

Small cosmetic change to settings page.
image

@MarcHagen MarcHagen changed the title Feature/remote header auth [feat] Remote header auth Jun 7, 2022
@louislam
Copy link
Owner

louislam commented Jun 9, 2022

I think this maybe a similar pull request: #474.

@Computroniks
Copy link
Contributor

#474 looks pretty inactive and would probably have a bunch of merge conflicts. This PR seems to be a bit more developed so I would be tempted to say go with this one when it is ready.

@MarcHagen
Copy link
Contributor Author

Yes, #474 is outdated and failed to fix some critical issues. This code is in theorie complete. I'm using this version internally right now.
I've created a draft to get a discussion going.
The base for this PR is indeed #474, but better implemented and more up-to-date

@sevmonster
Copy link

I think this would be a good time to implement #1672? I personally have no need for native authentication and feed everything through authentik, it would be easier for future users to be able to simply disable it from the start.

I will look over these changes and try them in my environment as well.

@MarcHagen
Copy link
Contributor Author

@sevmonster at the moment the code in this PR still relay on the internal user.

And I had too change the default admin to my own username to get it working with an SSO.
(Hence the change username script)

I'm not sure if we can bypass the whole internal user thing as it is used by some mechanisms that relay on some info like a userId or userName.

This is why this is also still a draft PR, any feedback on that is absolutely welcome!

@louislam
Copy link
Owner

Just a reminder, if the feature is finished, remember to click "Ready for Review".

@MarcHagen
Copy link
Contributor Author

MarcHagen commented Sep 19, 2022

@louislam Too me this can be implemented, but SSO is mainly based on multi user.
So this will work for instance with an SSO with only 1 account. But SSOs with multiple users will fail as Uptime Kuma doesn't have multi user support and cannot find that user.

The other side it to disable all Uptime Kuma user management when using SSO. (introducing "multi user").
Thats some big steps/changes, and im not the person to make that call.

I'm happy to update and test to get it up-to-date with other features. But then it needs a sidenote about the username change and that it only works with 1 user SSO.

@MarcHagen MarcHagen force-pushed the feature/remote-header-auth branch 2 times, most recently from 5093c9e to c14e425 Compare December 11, 2022 20:57
@MarcHagen MarcHagen force-pushed the feature/remote-header-auth branch 2 times, most recently from 4fe582c to 5d27911 Compare December 23, 2022 22:22
@MarcHagen MarcHagen force-pushed the feature/remote-header-auth branch 2 times, most recently from dbcf008 to 9c6ec48 Compare March 31, 2023 21:12
@MarcHagen MarcHagen force-pushed the feature/remote-header-auth branch 4 times, most recently from 4f26422 to a60090a Compare November 11, 2023 21:55
@CommanderStorm CommanderStorm added area:user-management blocked cannot progress because of external reasons pr:needs review this PR needs a review by maintainers or other community members pr:needs other pr merged pending other PR to be merged first labels May 19, 2024
@CommanderStorm
Copy link
Collaborator

CommanderStorm commented May 19, 2024

The same as #4751, this is blocked by #3571 which has slotted into the v2.2 release.

The current v2.0 release is tracked in #4500

@github-actions github-actions bot added the pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again label May 19, 2024
@github-actions github-actions bot added pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again and removed pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again labels May 19, 2024
@MarcHagen
Copy link
Contributor Author

I’ll update the branch.
Since opening this PR I’m using SSO (Authelia) for my own instance. Never had issues 😁.

@github-actions github-actions bot added pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again and removed pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again labels May 21, 2024
@github-actions github-actions bot removed the pr:please resolve merge conflict A merge-conflict needs to be addressed before reviewing makes sense again label May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:user-management blocked cannot progress because of external reasons pr:needs other pr merged pending other PR to be merged first pr:needs review this PR needs a review by maintainers or other community members
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants