Added group_id to authorization #354

ddey2 · 2023-02-23T22:12:50Z

Added list of 'group_ids' (ObjectIds) to Authorization collection in Mongo
Each dataset has one entry in Authorization collection with a list of group_ids and a list of users.
Dataset_id is now set as Object Id
Associated changes to retrieve role etc in routers/authorization.py
Added checks to remove duplicates in list in routers/groups and routers/authorization

max-zilla · 2023-02-28T14:47:32Z

I reviewed the discussion on Slack and this PR. I was expecting to see user_ids removed from authorization and only a list of group_ids. Then each user would get their own personal group as we discussed, and we could store that group_id in the user object as well so we can check current_user.group_id in authorization.group_ids. We would not maintain a separate list of user_ids.

One concern I pointed out is that if I create the dataset (added to user_ids) and then I'm in a group that is added to the dataset (which would've added my user_id again, in this case it wouldn't be duplicated) and we remove that group, it is difficult to make sure my user_id is NOT removed from the list because I had a role before the group was added. If I understand correctly, in this implementation we wouldn't track that and I'd potentially get removed from ownership.

This would also involve adding a flag to the group model is_personal or something that means we can't add other people to my personal group. Let me know what you think and if I'm misunderstanding.

ddey2 · 2023-02-28T20:55:47Z

@max-zilla As mentioned in slack, we decided to go with this implementation for now. It's subject to change in future based on requirements. Keeping this in mind, could you review the PR? I think this will change a couple of the things like your PR #357

lmarini

Left some small comments and one bigger one regarding the interaction of list of users and list of groups and how we query them.

backend/app/routers/authorization.py

longshuicy · 2023-03-02T16:06:56Z

backend/app/models/authorization.py

-    dataset_id: str
-    user_id: EmailStr
+    dataset_id: PyObjectId
+    user_ids: List[EmailStr] = []


Ah... we are doing a list of user_ids now? this might break the view :-( cuz I was assuming this is just one string

Yes, user_ids and groups_ids all are list.

We might need to change the view then.

If we have decided to make user_ids a list, then I'll need to update some of the logic to filter user access. Will convert this into an issue.

* Added new component for file actions * Tested file download and file delete from File page * Verified after deleting file, page navigates back to main dataset page * Fixed version-not-updating error * Updated onSave sequence to first synchronously update the file and then subsequently call listVersions API to display on the frontend * Fixed lint issues by running eslint

* Static UI for displaying logs on extractors * Created 2 React custom components: * ExtractorStatus -- UI table to display summary of job/progress so far * ExtractorLogs -- terminal-like UI to display logs in real-time as they are being fetched * [WIP] Enable extractor job id retrieval from backend * Updated redux store value when job id is returned * Added actions for job update, job summary API endpoints * [WIP DO NOT MERGE] Testing frontend functionality with dynamic log rendering * Updated API endpoints, return payload, reducers and actions (#330) * Updated API endpoints, return payload, reducers and actions for the extractor jobs * Tested API calls to fetchJobSummary, fetchJobUpdates from frontend, able to receive extractor job data * Tested submit job process flow, able to retrieve extractor job id from the backend * Removed redundant code --------- Co-authored-by: Chen Wang <[email protected]> * Integrated UI with backend * Tested with sample extractor logs, able to view both extractor status and summary from the UI * Removed hardcoding of job id * Fixed indentation * Adding download button on UI to download correct verison file (#334) * Adding download button on UI to download correct verison file * fix filename issue * Update FileVersion.ts --------- Co-authored-by: Chen Wang <[email protected]> * fix member typing (#336) * fix member typing * adjust the test so it can pass * fixing error of blank page on submit file to extractor (#338) files.py - renaming resubmit method codegen fixing error in listeners.py * Modified extractor job summary duration calculation * Moving fetchJobSummary, fetchJobSummary API calls to ExtractorStatus component to fix interval issue --------- Co-authored-by: Chen Wang <[email protected]> Co-authored-by: Dipannita <[email protected]> Co-authored-by: Todd Nicholson <[email protected]>

* various fixes to message listener * black formatting * Added new component for file actions (#352) * Added new component for file actions * Tested file download and file delete from File page * Verified after deleting file, page navigates back to main dataset page * Fixed version-not-updating error * Updated onSave sequence to first synchronously update the file and then subsequently call listVersions API to display on the frontend * Fixed lint issues by running eslint * UI for displaying logs on extractors (#317) * Static UI for displaying logs on extractors * Created 2 React custom components: * ExtractorStatus -- UI table to display summary of job/progress so far * ExtractorLogs -- terminal-like UI to display logs in real-time as they are being fetched * [WIP] Enable extractor job id retrieval from backend * Updated redux store value when job id is returned * Added actions for job update, job summary API endpoints * [WIP DO NOT MERGE] Testing frontend functionality with dynamic log rendering * Updated API endpoints, return payload, reducers and actions (#330) * Updated API endpoints, return payload, reducers and actions for the extractor jobs * Tested API calls to fetchJobSummary, fetchJobUpdates from frontend, able to receive extractor job data * Tested submit job process flow, able to retrieve extractor job id from the backend * Removed redundant code --------- Co-authored-by: Chen Wang <[email protected]> * Integrated UI with backend * Tested with sample extractor logs, able to view both extractor status and summary from the UI * Removed hardcoding of job id * Fixed indentation * Adding download button on UI to download correct verison file (#334) * Adding download button on UI to download correct verison file * fix filename issue * Update FileVersion.ts --------- Co-authored-by: Chen Wang <[email protected]> * fix member typing (#336) * fix member typing * adjust the test so it can pass * fixing error of blank page on submit file to extractor (#338) files.py - renaming resubmit method codegen fixing error in listeners.py * Modified extractor job summary duration calculation * Moving fetchJobSummary, fetchJobSummary API calls to ExtractorStatus component to fix interval issue --------- Co-authored-by: Chen Wang <[email protected]> Co-authored-by: Dipannita <[email protected]> Co-authored-by: Todd Nicholson <[email protected]> * Millisecond (#360) * it's seconds * add more icons * default to open tab --------- Co-authored-by: Aruna Parameswaran <[email protected]> Co-authored-by: Chen Wang <[email protected]> Co-authored-by: Dipannita <[email protected]> Co-authored-by: Todd Nicholson <[email protected]>

* context is now changed to list union of AnyUrl or dict. This should make v2 compatible with some extractors from extractors-core which dynamically build the context. * formatting fix test * formatting * still getting pytest errors changed context from optional[list] to list with default value empty, but still is not fixing the issues. * should fix the tests * formatting * getting rid of class 'context element' not needed * getting rid of class 'context element' not needed * removing console log --------- Co-authored-by: Chen Wang <[email protected]>

* add a readme * add listener job update view * temp * add init * add to production docker-compose as well * 343 list resources that user has access (#355) * add logic to filter user * add filters to file * list files working too * add same filter for folder * include executions * exempt the current resource owner * add test database * rewrite createView part * rewrite init script but it's still not working yet * fix typo * fix linting

…on refresh issue (#365) * Tested dataset search by clicking button and pressing enter, able to view search results

…-in-mongo

Added group_id to authorization

1e8c563

ddey2 requested a review from max-zilla as a code owner February 23, 2023 22:12

ddey2 linked an issue Feb 23, 2023 that may be closed by this pull request

Add group Id to Authorization collection in Mongo #344

Closed

ddey2 requested review from lmarini, longshuicy and tcnichol February 23, 2023 22:13

Added list of group_ids instead of one group_id per entry

addfb96

lmarini requested changes Mar 1, 2023

View reviewed changes

backend/app/routers/authorization.py Outdated Show resolved Hide resolved

backend/app/routers/authorization.py Show resolved Hide resolved

backend/app/routers/authorization.py Outdated Show resolved Hide resolved

backend/app/routers/authorization.py Outdated Show resolved Hide resolved

Addressing comments

ad4b329

ddey2 requested a review from lmarini March 1, 2023 22:58

fixing lint failure

d93101e

max-zilla approved these changes Mar 2, 2023

View reviewed changes

ddey2 mentioned this pull request Mar 2, 2023

add metadata auth framework. #359

Merged

longshuicy reviewed Mar 2, 2023

View reviewed changes

longshuicy mentioned this pull request Mar 2, 2023

Update logic in get resources since user_ids will be list #367

Open

longshuicy approved these changes Mar 2, 2023

View reviewed changes

arunapa and others added 10 commits March 2, 2023 10:21

Replaced EmbeddedSearch with custom search box component to fix sessi…

20cd028

…on refresh issue (#365) * Tested dataset search by clicking button and pressing enter, able to view search results

Merge branch 'main' into 344-add-group-id-to-authorization-collection…

0475018

…-in-mongo

match user_ids which is a list now (#368)

7e54a7c

Merge branch 'main' into 344-add-group-id-to-authorization-collection…

a4da092

…-in-mongo

update to lookup query

69d42f2

longshuicy mentioned this pull request Mar 6, 2023

Wrong dataset URL throws 500 instead of 404 #380

Closed

update authorization_deps queries

8dcd8c3

max-zilla added 2 commits March 6, 2023 13:16

simplify group logic in auth check

b35d986

formatting

f5e194d

max-zilla mentioned this pull request Mar 6, 2023

Improve group management logic #384

Closed

Formatting

65a08de

max-zilla merged commit d560536 into main Mar 6, 2023

max-zilla deleted the 344-add-group-id-to-authorization-collection-in-mongo branch March 6, 2023 20:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added group_id to authorization #354

Added group_id to authorization #354

Uh oh!

ddey2 commented Feb 23, 2023 •

edited

Loading

Uh oh!

max-zilla commented Feb 28, 2023

Uh oh!

ddey2 commented Feb 28, 2023

Uh oh!

lmarini left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

longshuicy Mar 2, 2023

Uh oh!

ddey2 Mar 2, 2023

Uh oh!

longshuicy Mar 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Added group_id to authorization #354

Added group_id to authorization #354

Uh oh!

Conversation

ddey2 commented Feb 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

max-zilla commented Feb 28, 2023

Uh oh!

ddey2 commented Feb 28, 2023

Uh oh!

lmarini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

longshuicy Mar 2, 2023

Choose a reason for hiding this comment

Uh oh!

ddey2 Mar 2, 2023

Choose a reason for hiding this comment

Uh oh!

longshuicy Mar 2, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ddey2 commented Feb 23, 2023 •

edited

Loading