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

Auth: Add support for single sign-on via OpenID Connect (OIDC) #782

Open
francescocarzaniga opened this issue Jan 1, 2021 · 46 comments
Open
Assignees
Labels
authentication User Account Management and Authentication idea Feedback wanted / feature request please-test Ready for acceptance test priority Supported by early sponsors or popular demand security Impact on server or browser security

Comments

@francescocarzaniga
Copy link

francescocarzaniga commented Jan 1, 2021

It seems that multi-user instances are on your radar, so I would like to suggest implementing OIDC for the login flow. OIDC is becoming the de-facto standard and there are tons and tons of projects (Open Source and not) using it now. It could be used together with LDAP with an IdP like Keycloak, so it would cover all the bases there and not require you to implement other Directory services.
I would personally suggest looking at Owncloud for inspiration (also for the Open Source/commercial side).


Related Issues:

Documentation:

@bnounours
Copy link

+1
For now either the collection is public (which is not ok for personal photos) either the admin account need to be connected (which is not ok to give admin account to all users)

@butonic
Copy link

butonic commented Jan 2, 2021

For OCIS we integrated konnectd from kopano as an OpenID Identity Provider. It can use an LDAP server to provide users. A minimal version could use glauth as the user backend. Personally, I'd love to see a reusable minimal set of services to provide users for open source projects that want to embrace OpenID Connect. It sucks to implement user authentication. Small deployments don't need a full blown keycloak server but should be able to embed a small scale solution. We did it with ownCloud and we did it again for ocis. It should be possible to flesh out something that can be reused by all. Konnectd and glauth are written in go and with ocis we tried to go for a microservice architecture. Maybe we could add Photoprism as one of the services? Happy to discuss ways forward. Ping me on https://talk.owncloud.com.

Cheers and Happy New year!

@lastzero lastzero added the idea Feedback wanted / feature request label Jan 3, 2021
@lastzero lastzero changed the title OpenID Connect Auth: OpenID Connect Jan 3, 2021
@Nicarim
Copy link

Nicarim commented Aug 28, 2021

Hey - got a question regarding this issue. Is this something you're considering implementing to the project? I might want to contribute the code to repository but I'd like to know how would you prefer to do it.

My initial idea (and reason) for adding SSO (or OpenID, or whatever) was to initially support reverse proxy functionality authentication middle like for example in Traefik authentication middlewares.

Good example of such configuration can be found here for authelia SSO.

I'd like to gate access to my private cloud through SSO solution like Authelia. They provide a way to inject HTTP header called Remote-User or whatever name you give it - so that application receiving request can trust that header and that user is authorized and their name is what is in that header. Example implementation of it with Jira out of their documentation.

Getting to the point - would you accept contribution to get Remote-User header be a way to authorize the user?

Perhaps initially could that be whatever is in that header is treated as admin user, but if there is no such header then you're prompted to log in?

Example Flow Unauthorized:

  • User enters example.com/photoprism
  • Traefik sees user is not authenticated by asking Authelia if their credentials in cookies / session are correct
  • User is redirected to independent flow of authentication provided by Authelia
  • Authelia completes authentication, redirects back to example.com/photoprism and injects header Remote-User: youruserfromSSO
  • PhotoPrism sees that header and automatically authenticates user as admin

Of course assumption is that this header is dropped if provided from the internet and it is only injected by trusted middleware.

Just to be sure - You can achieve this functionality right now by setting the photoprism instance to public, but that removes authentication for other endpoints like WebDAV endpoint which makes it impossible to have public instance with authorization on that endpoint. I had an idea of making exception for WebDAV endpoint that it would accept PhotoPrism native credentials, and that visiting through browser normally would use SSO. For that to happen - above idea needs to be somehow implemented.

Tell me your thoughts about it :)

Not sure if I should post it under OpenID or separate issue, so if you want me to create new issue I can do that.

@lastzero lastzero added in-progress Somebody is working on this and removed unfunded labels Aug 28, 2021
@lastzero
Copy link
Member

Already working on it.

@Nicarim
Copy link

Nicarim commented Aug 28, 2021

Full OpenID authenication or just the trusted header option?

@moximoti
Copy link
Collaborator

Hi, We’re currently implementing user management and OpenID Connect Client (RP) in PhotoPrism. This does not include trusted headers. I suggest to wait until user management and OIDC are properly implemented before aiming for a third authentication method, as we need to integrate all this with our current session management.

Btw, Authelia also supports OIDC. So there’s actually no need to implement trusted headers for your use case once we have OIDC ;)

@Nicarim
Copy link

Nicarim commented Aug 28, 2021

That's correct - OIDC will work just as well if there is OIDC.

I just thought that if you wouldn't want to waste too much time on that trusted headers could be a good compromise - after all it would be just some IF somewhere granting unconditional access instead of checking password.

Just a question then - what about the WebDAV scenario where I still want to be able to use PhotoPrism native credentials? Or do you have some other solution to that problem? if OIDC will be used then the application would need to be able to get the authentication somehow through OIDC - and none of the apps like photosync would support that I guess.

@lastzero
Copy link
Member

We should provide app passwords / tokens for that.

moximoti added a commit that referenced this issue Sep 9, 2021
moximoti added a commit that referenced this issue Sep 9, 2021
moximoti added a commit that referenced this issue Sep 9, 2021
moximoti added a commit that referenced this issue Sep 9, 2021
moximoti added a commit that referenced this issue Sep 9, 2021
moximoti added a commit that referenced this issue Sep 9, 2021
moximoti added a commit that referenced this issue Sep 20, 2021
moximoti added a commit that referenced this issue Sep 20, 2021
moximoti added a commit that referenced this issue Sep 20, 2021
moximoti added a commit that referenced this issue Sep 20, 2021
moximoti added a commit that referenced this issue Sep 29, 2021
If username exists locally, append suffix to preferred username
moximoti added a commit that referenced this issue Oct 23, 2021
lastzero added a commit that referenced this issue Jun 28, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
graciousgrey added a commit that referenced this issue Jun 28, 2024
lastzero added a commit that referenced this issue Jul 1, 2024
Requires further testing and refinement before it can be released.

Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

lastzero commented Jul 1, 2024

This commit implements the GET /api/v1/oidc/redirect endpoint, so authentication against OIDC Identity Providers should generally work. Note that further refinement/testing is needed before we can release this, and that the PHOTOPRISM_OIDC_REDIRECT config option is not yet implemented, i.e. setting this option has no effect yet.

The preview build on Docker Hub has been updated so you can help test what is already implemented:

@lastzero lastzero changed the title Account: Add Support for OpenID Connect (OIDC) Auth: Add Support for Single Sign-On via OpenID Connect (OIDC) Jul 1, 2024
@twsouthwick
Copy link

I gave this a test and it works great with Authelia. Thanks so much for this - much easier to add friends/family who are already in my SSO. A couple points of feedback:

  • Users are automatically added to "guest". Are the granular roles going to continue to be under the membership versions? Either way, it would be nice to have a way to map OIDC roles/groups to a photoprism group with the auto-register. For now, the cli works, but this is a nice to have
  • PHOTOPRISM_OIDC_WEBDAV=true set the WebDAV to "Disabled" - I couldn't tell from the docs if it's expecting a boolean or something else

Overall, much appreciated functionality! One less password to remember.

@twsouthwick
Copy link

As a follow up suggestion, things like PHOTOPRISM_OIDC_WEBDAV would make a lot of sense to be PHOTOPRISM_OIDC_WEBDAV_ROLE or something similar where you supply the OIDC role so not all users are automatically allowed webdav access

lastzero added a commit that referenced this issue Jul 2, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

lastzero commented Jul 2, 2024

Thanks for testing it, @twsouthwick! That's very helpful.

Users are automatically added to "guest". Are the granular roles going to continue to be under the membership versions?

We considered giving the ability to register higher privileged accounts, either through a config option or OIDC claims as you can see in this feature branch we used for testing:

isAdmin, claimErr := oidc.HasRoleAdmin(userInfo)
if claimErr == nil {
u.RoleAdmin = isAdmin
log.Debug("photoprism_admin: ", isAdmin)
} else {
log.Debug(claimErr)
}

However, there are some scenarios where this could lead to malicious or accidental privilege escalation, e.g. when inexperienced users use Google as Identity Provider and set the default role to "admin".

At this point, I don't believe the convenience gain for users having a custom identity provider (so they have full control over who uses it and who can sign up) is worth the risk - especially if it's just a handful of users where you need to change the role after they've created an account. Would you agree with that assessment?

PHOTOPRISM_OIDC_WEBDAV=true set the WebDAV to "Disabled" - I couldn't tell from the docs if it's expecting a boolean or something else

For a list of accepted bool values, see (it accepts everything that makes sense e.g. 0, 1, true, false,...):

As a follow up suggestion, things like PHOTOPRISM_OIDC_WEBDAV would make a lot of sense to be PHOTOPRISM_OIDC_WEBDAV_ROLE or something similar where you supply the OIDC role so not all users are automatically allowed webdav access

For security reasons, guest users cannot get WebDAV access or any other type of direct file system access. So, as mentioned in the usage description, this flag only has an effect if the user role allows it in the first place, e.g. when the account role is later changed to "user" or "admin" - see the "Roles and Permissions" overview in our User Guide:

Basically, the PHOTOPRISM_OIDC_WEBDAV flag is just there so we don't have to make that choice for everyone, as some might assume it's enabled and others might assume it's disabled by default. This way it's transparent and users can't blame us for not having known about this option, e.g. when they give someone "user" rights but don't want them to have webdav access as well.

graciousgrey added a commit that referenced this issue Jul 2, 2024
lastzero added a commit that referenced this issue Jul 2, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Jul 2, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Jul 2, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Jul 2, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Jul 2, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

lastzero commented Jul 2, 2024

With today's changes, the OIDC config options (as shown above) should be fully functional. Much of the profile information provided by OIDC should now also be set when you sign up, and updated the next time you sign in (unless you have e.g. manually changed your display name in PhotoPrism).

Unless there are essential features missing, I would suggest keeping this feature scope so we can shift our focus to testing and release this as soon as possible... any help with that is much appreciated! 💎

@lastzero lastzero added please-test Ready for acceptance test and removed in-progress Somebody is working on this labels Jul 2, 2024
@lastzero lastzero removed their assignment Jul 2, 2024
lastzero added a commit that referenced this issue Jul 2, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Jul 2, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Jul 2, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
lastzero added a commit that referenced this issue Jul 3, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero lastzero changed the title Auth: Add Support for Single Sign-On via OpenID Connect (OIDC) Auth: Add support for single sign-on via OpenID Connect (OIDC) Jul 3, 2024
@twsouthwick
Copy link

Thanks for the response. I've signed up for the essential level to try out the different user roles now that I can easily onboard my family.

However, there are some scenarios where this could lead to malicious or accidental privilege escalation, e.g. when inexperienced users use Google as Identity Provider and set the default role to "admin".

At this point, I don't believe the convenience gain for users having a custom identity provider (so they have full control over who uses it and who can sign up) is worth the risk - especially if it's just a handful of users where you need to change the role after they've created an account. Would you agree with that assessment?

I can see the point, but I'd prefer to have it tied to a claim and ensure the docs are clear. However, I would actually propose a different pattern, I think, then what you have above, to allow users to define their own role mapping which would reduce the chances of this:

  1. Define a set of variables for role mapping, i.e. PHOTOPRISM_OIDC_ADMIN_ROLE, PHOTOPRISM_OIDC_USER_ROLE, etc
  2. Identify precedence order. Regarding your concerns about inexperienced users, here you might go with the most restrictive group they're in; that is, if they're in the role for both admin and user, go with user. I'd prefer least restrictive (i.e. admin), but that's your call
  3. If none of the role mappings are defined, default to Guest

This way, there's not a "default" role being added, but will check the groups retrieved from OIDC and the chance of that inexperienced user falling into an unintended state of disclosure becomes much smaller, as they would have to set the env variable AND add that role to the user.

As a bonus, it would also be great if this were to reevaluate a user's role when signing in (or at least when a new token is submitted)

lastzero added a commit that referenced this issue Jul 4, 2024
Signed-off-by: Michael Mayer <michael@photoprism.app>
@lastzero
Copy link
Member

lastzero commented Jul 4, 2024

@twsouthwick Thanks for you support! Based on your comments, I assume you are using a self-hosted identity provider? If so, which one? It would be good to learn a bit more about your specific environment to better understand the context and potential attack vectors, and then compare possible solutions in terms of complexity and security. You are welcome to email us for this (or open a GitHub Discussion) as the comments under this issue can easily get lost due to their sheer number.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authentication User Account Management and Authentication idea Feedback wanted / feature request please-test Ready for acceptance test priority Supported by early sponsors or popular demand security Impact on server or browser security
Projects
Status: Preview 🐳
Development

No branches or pull requests