fix: overwriting what projects users have been assigned every time you login via oauth2

[neoReuters]

Overview

This pull request addresses an issue where logging in via the OAuth2 filter would overwrite the projects a user is assigned to, replacing them with the default project. This behavior was particularly undesirable for users with multiple project assignments, as it caused them to lose their existing projects. The fix ensures that existing projects are preserved, and the default project is only appended if it is not already present in the datashare list.

Changes

1. Check for Existing groups_by_applications and datashare: The logic now checks if groups_by_applications already exists in the user's data and reuses it rather than overwriting it. If the datashare list exists, it appends the oauthDefaultProject only if it doesn't already exist.
2. Preserving Existing Projects: Instead of overwriting the list of projects in datashare, this fix ensures that additional projects are appended, preserving any projects that the user was already assigned to.

Motivation

The issue was identified when logging in via the OAuth2 filter, where the user’s assigned projects were being overwritten with the default project. This was especially problematic for users with multiple project assignments, as they would lose their previously assigned projects each time they logged in. This fix provides a more flexible and reliable approach, ensuring that projects are appended rather than overwritten, preserving the existing assignments.

Impact

These changes are backward compatible with existing configurations and will not alter the current behavior unless there is an issue with project overwrites. It improves the system’s data integrity, ensuring that multiple projects can be tracked without losing existing assignments.

Request for Feedback

I would appreciate feedback on the overall approach for preserving and appending to datashare. Specifically, I’d welcome any suggestions for further optimizations or concerns about edge cases where existing project data could still be affected.

[bamthomas]

Hello @neoReuters

First, thanks for your PR. It took us some time to understand why this was done like it is.

So to give you some context:

• the source of the update was probably the api key epic: [DaaP API] generate and get one API key from the command line #504 we wanted to do ACL on projects with the api key
• we stored the users in a redis cache and use this cache to find the project keys
• so the source of "trust" is still the Identity Provider. That's why the user's cache is overridden at each login (the ACL could have been updated in Identity Provider)

Now if I understand your need:

• you put project ACL's for datashare in Redis or a Database
• you use AWS Cognito as identity provider (you mentioned it in feat: custom classes for authUsersProvider & authFilter + new auth filter class for AWS ALB + Cognito #1393 ) for authenticating your users
• each time they login they erase the project list with the default oauth project

(for reference see also the refactor made after the previous PR #1395 )

Is that correct?

[bamthomas]

so would this story fix your issue?
#1585

[neoReuters]

Hi @bamthomas - your assumptions are correct. Just for further brevity (not sure that it matters); we are using UsersInDB authUsersProvider over UsersInRedis. I tried both initially to see if it was an issue with the authUsersProvider but found it was something baked into the oauth2CookieFilter class instead.

To my understanding now after you've broken it down - the way the system works; it expects the list of projects to essentially be part of the identity provider's responsibility. So, in a sense, we must send through something like groups_by_applications.datashare as part of our token - I'm guessing this is the ideal behaviour?

As a solution then, I guess it would be helpful if we could have a custom key, kind of like what we did with oauthClaimAttribute, so that it can be adjusted as such, read when going through the processOAuthApiResponse function, and perhaps also it should be documented to help people understand how it works alongside maybe with how you should go about creating new projects (currently this involved us creating projects in the project table, which we then mapped to users in the user_inventory table) in server mode in general.

The current solution proposed assumed that it was Datashare's responsibility to map projects to users hence why we're adjusting this inside the user_inventory table. How do you think we should proceed with this? Open to ideas!

[neoReuters]

so would this story fix your issue? #1585

Ah just replied before seeing your reply, I guess something like this would work fine mostly. That means essentially the identity provider, in our case Cognito, is in charge of enriching the token with projects that user is responsible for, correct?

Funnily enough, before quite recently it was quite tough to do this, but AWS have finally provided a way to enrich tokens with a Lambda trigger (https://aws.amazon.com/about-aws/whats-new/2023/12/amazon-cognito-user-pools-customize-access-tokens/). So I can imagine it working something like this - we have something like a DynamoDB or even a relational database which holds the projects the users are mapped to. Then we fire the lambda to go and check and enrich the token with this user's projects.

So just some more observations. This works well but would also be interested in seeing whether you can also possibly provide a solution similar to the code this PR has, for people who might not have the ability to manipulate their oauth tokens? Thoughts @bamthomas?