georchestra / georchestra-gateway

GNU General Public License v3.0
0 stars 6 forks source link

local account creation for user connected with external identity provider #45

Closed marwanehcine closed 1 year ago

marwanehcine commented 1 year ago

On this PR, we are creating a new LDAP account for user connected using external IDP (Google, Mel, FranceConnect). When user connect for first time, a new LDAP account is created. After that, user information (firstname, lastname...) will be read from user LDAP account. External IDP users can modify password and connect using login/password page. For FranceConnect, an EndSessionURL is added to perform correct logout process. Create e new LDAP account is optional depending on property "georchestra.gateway.security.createNonExistingUsersInLDAP"

emmdurin commented 1 year ago

With FranceConnect, e-mail address should not be used as a unique identifier, as same user may have a different e-mail address depending on the FranceConnect identity provider he chooses to login using FranceConnect. So we should use another unique identifier containing identity provider name and OAuth2 subject value, which I propose in this commit, so that such a user will not have 2 different Georchestra account.

The problem with that is if the user login using OAuth2 with 2 different providers with same e-mail address associated, second account creation will fail because even if UID does not exists, there is another account that has the same e-mail address. We may address this problem by searching for an existing account by UID or by e-mail, taking the one which answers positively, or create a new account if all two answers negatively.

I also think that we should treat theses automatically created accounts differently on some cases. The user should not be able to see his login UID, to change password, or to login using login/password form. Header should also display something specific to this case, but I do not know what at the moment. We may address this by adding a new sec-XXXXX header to forwarded requests so that services know that this is an auto-generated account.

groldan commented 1 year ago

hey sorry I just stumbled upon this pr right now. My first comment is I find it difficult to reason about it. Please add a meaningful commit message and PR description. Aside, I don't think creating accounts should be the responsibility of the gateway. Now, without a deeper understanding of the pr's intent and the fact that's 23:30 already here I don't feel capable of providing a better feedback, but at least let's discuss it and see if we can find an alternative.

marwanehcine commented 1 year ago

Hello @groldan, sorry if you find PR difficult to undrestand. The goal of PR is to create local LDAP account for users who connect using Google, Mel, FranceConnect. From second user login, users information will be loaded from LDAP account. Will be waiting for your questions/feedbacks. Thanks

marwanehcine commented 1 year ago

Hello @pmauduit , I have add option "createNonExistingUsersInTheLDAP" to enable local account creation, however when createNonExistingUsersInTheLDAP=false, what about "userDetails" page, this page cannot be shown unless a local account is created and also items was not shown to user since the is no local account. We need to handle these cases if we will go forward making local account creation optional. Thanks

pmauduit commented 1 year ago

Aside, I don't think creating accounts should be the responsibility of the gateway.

@groldan why not ? The gateway is the "entrypoint" for accessing the application, it sounds as the only place being aware of who is connecting how (when it deals with user authentication / authorization & external identity providers). If we want the administrators to be able of managing external users to e.g. ban an account, promote the user to more than ROLE_USER, ... we have to keep track of them, and one solution which sounded natural to us was to create the account into the openLDAP (which is the actual reference database for our users). I think that most online services which provide the possibility to use external 3rd party identity providers do function the same way (thinking of strava, but probably applies to others, they do keep my users info attached to my google account).

We are opened to discussion if you have a more relevant approach in mind