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

Implement OIDC #1701

Closed
wants to merge 1 commit into from
Closed

Implement OIDC #1701

wants to merge 1 commit into from

Conversation

m-baertschi
Copy link

This is a simple OIDC implementation. It's very basic and just logs the user in. Access control needs to be done on the IDP side.

@knadh
Copy link
Owner

knadh commented Jan 31, 2024

Thanks @m-baertschi! I'll review this over the weekend.

@knadh knadh added the enhancement New feature or request label Jan 31, 2024
@waza-ari
Copy link

Thanks for your work! Just want to advise that depending on the IDP you're using the guiding principle there might be that access control should be done on the application level (Keycloak being a prominent example). That being said, I'd rather implement a workaround in those providers than not having OIDC at all

@MaximilianKohler
Copy link
Contributor

I'm curious what the use case is for this?

@waza-ari
Copy link

I'm curious what the use case is for this?

Lets say you setup listmonk for any kind of organization - whether its a company, a non-profit or anything else for that matter - they usually have their identities (user accounts) managed in one central place (usually called an Identify Provider). Could be Azure AD, could be Keycloak, could be any IdP really.

Instead of having local user accounts (which need to be properly managed, e.g. deactivated if a user leaves the org) and also for the user to have one account / password only, OIDC allows to delegate authentication (WHO is the user) to the external system. Authorisation (WHAT this user can do, e.g. permission management) can then rely on this identity.

@waza-ari
Copy link

@m-baertschi how does the API work when OIDC is implemented?

@m-baertschi
Copy link
Author

OIDC just replaces the BasicAuth middleware if configured. I have not tested the API separately, but i will look into it. It is probably possible to check if the authorization header is present and perform basic auth.

This is a simple OIDC implementation. It's very basic and just logs the user in. Access control needs to be done on the IDP side.
@m-baertschi
Copy link
Author

The API now works with the admin credentials, even if oidc is configured. The Application now checks if the basic auth header is present and then validates it.

But this also means that if the docker container is configured with environment variables the default admin login still work. To fix this, the config file should not include any credentials at all.

@kSzajo
Copy link

kSzajo commented Mar 5, 2024

Could you add readme? I'd like to test it

@knadh
Copy link
Owner

knadh commented Mar 10, 2024

@m-baertschi, apologies for the delay. The implementation looks good!

  1. The coreos OIDC lib vs. https://github.com/zitadel/oidc, have you evaluated the latter? Just wanted to figure the lightest one as the coreos lib somehow adds ~300KB to the build.

  2. The Settings UI for the OIDC credentials is yet to be done, right? I can work on this and add it to your PR.

@m-baertschi
Copy link
Author

  1. I have not evaluated the zitadel oidc library. But it has a lot more imports in the go.mod then the coreos lib, so i don't think it will be smaller.
  2. Yes, there is no settings UI or documentation.

@m-baertschi
Copy link
Author

@kSzajo There are 3 options you need to set. The provider is the URL of your IDP for example: https://id.example.com
The client id and secret are from you registration of the app in your idp.

Environment variable (Docker):

  • LISTMONK_app__oidc__client_id
  • LISTMONK_app__oidc__client_secret
  • LISTMONK_app__oidc__provider

@knadh knadh self-assigned this Mar 12, 2024
@titansmc
Copy link

So have you also implemented multi-user ? In the sense that I log in to listmonk and start writing an email, and then someone else logs in as well, would they see the email I am writing ? or it would be a totally different session?

@knadh
Copy link
Owner

knadh commented Mar 12, 2024

@titansmc no, that's not possible without listmonk getting a built in user/permission system which it currently doesn't have.

@titansmc
Copy link

so no multi user, does it mean that everyone with login details will be able to see the email others have wrote ? So if someone writes a campain and saves it, would the rest see it?

@knadh
Copy link
Owner

knadh commented Apr 2, 2024

I've ended up extending this OIDC implementation towards multi-user support. Will see if we can have list-based ACLs on top of it in the upcoming version.

@knadh knadh mentioned this pull request Apr 2, 2024
@matmair
Copy link

matmair commented Apr 2, 2024

@knadh are you aware of polar.sh for Open Source funding? I would like to contribute a few dollars for this PR as it will help me with my SSO-everywhere goal.

@knadh
Copy link
Owner

knadh commented Apr 2, 2024

Thank you for the kind offer @matmair, but I'm not looking for funding. We should have multi-user + SSO support in the next release.

@knadh
Copy link
Owner

knadh commented Jun 22, 2024

Closing this PR as the work in progress here #543 now supersedes it. Thank you for kick starting this @m-baertschi

@knadh knadh closed this Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants