reva icon indicating copy to clipboard operation
reva copied to clipboard

Add authentication provider field in the token

Open gmgigi96 opened this issue 2 years ago • 8 comments

gmgigi96 avatar Sep 01 '21 14:09 gmgigi96

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

update-docs[bot] avatar Sep 01 '21 14:09 update-docs[bot]

Why do you need the auth_provider in the token? Is it an address, or just an arbitrary name? Please give some insight in what you are trying to do!

butonic avatar Sep 03 '21 06:09 butonic

Hi @butonic, the goal with this is to be able to know from the token payload how the user was authenticated. We need this as we have some auth mechanism (like the machine type) that allows to impersonate the user and today you cannot differentiate if the token was generated by the physical user or because of an impersonation without scrapping logs. This helps for auditing purposes.

The auth_type value is just the driver used to authenticate: basic, bearer and so on.

labkode avatar Sep 07 '21 06:09 labkode

@butonic please review as I'll like this one to be part of the new release for OCIS.

labkode avatar Sep 13 '21 06:09 labkode

@butonic

micbar avatar Sep 13 '21 12:09 micbar

Hi @butonic, the goal with this is to be able to know from the token payload how the user was authenticated. We need this as we have some auth mechanism (like the machine type) that allows to impersonate the user and today you cannot differentiate if the token was generated by the physical user or because of an impersonation without scrapping logs. This helps for auditing purposes.

Hm, especially for impersonating users I would expect the identities of both: the impersonator and the user. That way every service can log both identities in an audit log. Furthermore, in the web ui you may want to show that the user is being impersonated, which also requires both identities in the token ... or a second token.

The auth_type value is just the driver used to authenticate: basic, bearer and so on. This will require aggregating logs via spans.

@labkode What do you think about putting both identities in the token?

What you wrote makes sense in case a user is impersonating another one, like an admin. In this case the impersonator is just a machine and does not involve an originating token. I'll create a GH issue to track that use-case which I think is quite handy, I remember Gitlab had a way for an admin to switch to any user to help debugging: https://github.com/cs3org/reva/issues/2068

butonic avatar Sep 14 '21 07:09 butonic

@labkode What do you think about putting both identities in the token?

I agree with this. We already have the user object in the token and I think that should indicate whether an admin is impersonating some other user. Maybe we can have a custom Id.OpaqueId for such cases, and also indicate this in the UserType field?

ishank011 avatar Sep 21 '21 08:09 ishank011

Sounds good, but probably modifying the UserType with a custom one can break something in the code, because this should be the one of the impersonated user. Why don't we do the same you proposed, but using the User.Opaque field, in which insert the user that impersonate?

On Tue, 21 Sep 2021, 10:29 Ishank Arora, @.***> wrote:

@labkode https://github.com/labkode What do you think about putting both identities in the token?

I agree with this. We already have the user object in the token and I think that should indicate whether an admin is impersonating some other user. Maybe we can have a custom Id.OpaqueId for such cases, and also indicate this in the UserType field?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cs3org/reva/pull/2031#issuecomment-923756632, or unsubscribe https://github.com/notifications/unsubscribe-auth/AJQYQQKHS5KSAO3JOFBOFNTUDA64ZANCNFSM5DGVEZVA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

gmgigi96 avatar Sep 22 '21 06:09 gmgigi96