-
-
Notifications
You must be signed in to change notification settings - Fork 419
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
Add keycloak authentication #240
base: master
Are you sure you want to change the base?
Conversation
We have defined some environment variables:
|
This sounds useful! A few notes:
|
keycloak.loadUserInfo().then(function(userInfo) { | ||
if (Tools.server_config.KEYCLOAK_USERINFO_ATTRIBUTE) { | ||
if (!userInfo[Tools.server_config.KEYCLOAK_USERINFO_ATTRIBUTE]) { | ||
alert("Sitema non disponibile per l'utente " + userInfo.preferred_username); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't have an "alert", and all messages should be localized
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not found anything to replace alert that you do not think about this: https://github.com/t4t5/sweetalert
- `KEYCLOAK_ENABLE` is used for enable OIDC authentication | ||
- `KEYCLOAK_URL` is the URL of Keycloak, eg, `https://keycloak-server/auth` | ||
- `KEYCLOAK_REALM` is the Realm name, eg, **myrealm** | ||
- `KEYCLOAK_CLIENTID` is the Public Client Id, eg, **myapp** | ||
- `KEYCLOAK_USERINFO_ATTRIBUTE` is the attribute name for authorization, and it is not mandatory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't have to take more than an optional oidc discovery url
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think it is necessary, everything is conditioned on the KEYCLOAK_ENABLE variable
Amy further improvements @mspasiano? I'm really looking forward to it ;-) |
We shouldn't have to take more than an optional oidc discovery url. Keycloak implements the standard oidc protocol; I don't think this should be keycloak-specific, mention a realm, or need a "userinfo" configuration. |
You could use this https://www.npmjs.com/package/openid-client what do you think? If you think it's appropriate I can take care of it, I did the same thing for PeerTube https://www.npmjs.com/package/peertube-plugin-oidc-cnr |
Yes, we can use an external lib. But we should keep compatibility with the existing jwt authentication mechanism described in https://github.com/lovasoa/whitebophir#authentication |
Has there been any progress with this? I would love to use OIDC authentication via authentik with WBO. |
Together with my colleague @GiorgioBart we have extended the authentication method to https://github.com/keycloak