Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Have permissions be filtered (DelegatedWork or DelegatedPersonal) based on account type #1008

Merged
merged 5 commits into from
Jul 12, 2021
Merged

Have permissions be filtered (DelegatedWork or DelegatedPersonal) based on account type #1008

merged 5 commits into from
Jul 12, 2021

Conversation

lhao03
Copy link
Contributor

@lhao03 lhao03 commented Jul 1, 2021 •
edited
Loading

Note: This is from an intern, but I forgot to add interns to my branch name.

Overview

I noticed when using Graph Explorer as a personal account (MSA) I still get shown DelegatedWork permissions. When I try to accept them I can't.

delegatedPerms

Demo

For instance,
image
a personal account should only see "User.Read".

What this bug fix shows:
image

What is currently shown (only DelegatedWork):
image

Notes

May want to change the "Not supported" thing that pops up.
image

Also I apologize for the PR I opened and then closed when I realized there were files I didn't want to merge.

Testing Instructions

No new tests were added because the function getProfileInfo is already being tested and it calls the new function I added. Currently all existing tests pass.

@lhao03 lhao03 marked this pull request as ready for review July 1, 2021 14:27
@lhao03 lhao03 changed the title working filter permissions Have permissions be filtered (DelegatedWork or DelegatedPersonal) based on account type Jul 1, 2021
thewahome
thewahome requested changes Jul 7, 2021
View reviewed changes
src/app/services/actions/permissions-action-creator.ts Show resolved Hide resolved
src/app/services/actions/profile-action-creators.ts Outdated Show resolved Hide resolved
src/app/views/authentication/profile/Profile.tsx
const betaJsonUserInfo = actions
? await actions.getProfileInfo({
selectedVerb: 'GET',
sampleUrl: BETA_USER_INFO_URL
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling the Beta Profile endpoint brings in a lot of new data. Consider manipulating the data received to fit in with the IUser type and update this to carry the extra data that is needed. It seems that the object change to the profile contents stored in state breaks the user by email sample query.

Copy link
Contributor Author

@lhao03 lhao03 Jul 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

d492b75, f0a0105 should fix the problem of overwriting the profile contents in state, so no manipulation of data is needed.

@lhao03
Copy link
Contributor Author

lhao03 commented Jul 8, 2021 •
edited
Loading

I've removed console.log(), the try catch and changed the code so that the profile contents in state does not get overwritten. I added a new function isBetaURLResponse that returns true or false and put the profileTypeSuccess in the existing if statement so the profile contents is not overwritten. I thought that might be better than mapping the beta url. Please let me know if this works!

@thewahome thewahome self-requested a review July 9, 2021 12:09
thewahome
thewahome approved these changes Jul 9, 2021
View reviewed changes
Copy link
Collaborator

@thewahome thewahome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works

@lhao03
Copy link
Contributor Author

lhao03 commented Jul 9, 2021

Thanks for your feedback and review @thewahome! Since I don't have permissions to merge the PR, would you be able to do that if the PR is ready? Thanks 😊!

@thewahome thewahome merged commit 88a2018 into microsoftgraph:dev Jul 12, 2021
@lhao03 lhao03 deleted the lhao03-filter-for-delegated-permissions branch July 12, 2021 13:25
@lhao03 lhao03 mentioned this pull request Jul 12, 2021
Closed
thewahome added a commit that referenced this pull request Jul 15, 2021
@thewahome
chore(release): 4.11.0 (#1030)
e6bc5f3 
* Fix: improve themes MessageBar colors (#1015)

* Feature: Have permissions be filtered (DelegatedWork or DelegatedPersonal) based on account type (#1008)

* Fix: url truncation (#1012)

* Fix: Response Preview truncation (#1017)

* Task: change sandbox URL (#992)
thewahome added a commit that referenced this pull request Jul 15, 2021
@thewahome
chore(release): 4.11.0 (#1031)
19733ac 
* Fix: url truncation (#1012)

* Fix: improve themes MessageBar colors (#1015)

* Fix: Response Preview truncation (#1017)

* Feature: Have permissions be filtered (DelegatedWork or DelegatedPersonal) based on account type (#1008)

* Fix: terminate process on entering multiple slash (#824)

* Task: change sandbox url (#992)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thewahome thewahome thewahome approved these changes

@adhiambovivian adhiambovivian Awaiting requested review from adhiambovivian

@Shjokie Shjokie Awaiting requested review from Shjokie

@ddyett ddyett Awaiting requested review from ddyett

@ElinorW ElinorW Awaiting requested review from ElinorW

@millicentachieng millicentachieng Awaiting requested review from millicentachieng

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lhao03 @thewahome