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.

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

Add initial workspace creator as workspace enterprise app owner #2627

Merged
merged 16 commits into from
Sep 29, 2022

Conversation

marrobi
Copy link
Member

@marrobi marrobi commented Sep 21, 2022

Resolves #2625

How is this addressed

  • Add initial workspace creator as owner of the enterprise app
  • Give Application Admin Directory.Read.All permissions in make auth

Breaking change.

The Application Admin account needs to be granted Directory.Read.All permissions:

image

@marrobi marrobi enabled auto-merge (squash) September 21, 2022 10:10
@github-actions
Copy link

github-actions bot commented Sep 21, 2022

Unit Test Results

0 tests   0 ✔️  0s ⏱️
0 suites  0 💤
0 files    0

Results for commit 58e10e4.

♻️ This comment has been updated with latest results.

@marrobi marrobi changed the title Add initial workspace owner as worksapce enterprise app owner Add initial workspace creator as workspace enterprise app owner Sep 21, 2022
@marrobi
Copy link
Member Author

marrobi commented Sep 21, 2022

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/3098465186 (with refid 4559838e)

(in response to this comment from @marrobi)

@marrobi marrobi marked this pull request as draft September 21, 2022 14:24
auto-merge was automatically disabled September 21, 2022 14:24

Pull request was converted to draft

@marrobi
Copy link
Member Author

marrobi commented Sep 21, 2022

It seems this need an additional permissions to be able to add this user. It worked in my environment, but failed for a customer. I have the some additional permissions - Directory.Read.All and Group.ReadWrite.All. I am guessing this operation tries to read the workspace owner object and hence needs Directory.Read.All

@marrobi
Copy link
Member Author

marrobi commented Sep 21, 2022

Directory.Read.All fixes the deployment, although user still cannot access the enterprise app n the portal

@marrobi
Copy link
Member Author

marrobi commented Sep 22, 2022

User also needs to have "Directory Reader" role in AD.

@marrobi marrobi marked this pull request as ready for review September 22, 2022 13:01
Copy link
Member

@martinpeck martinpeck left a comment

Choose a reason for hiding this comment

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

Approved, but you have a typo

docs/tre-admins/identities/application_admin.md Outdated Show resolved Hide resolved
@marrobi
Copy link
Member Author

marrobi commented Sep 29, 2022

/test

@marrobi marrobi enabled auto-merge (squash) September 29, 2022 11:36
@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/3150965693 (with refid 4559838e)

(in response to this comment from @marrobi)

@marrobi
Copy link
Member Author

marrobi commented Sep 29, 2022

/test-force-approve passed https://github.com/microsoft/AzureTRE/actions/runs/3150965693

@github-actions
Copy link

🤖 pr-bot 🤖

✅ Marking tests as complete (for commit 58e10e4)

(in response to this comment from @marrobi)

@marrobi marrobi merged commit 0df13fd into microsoft:main Sep 29, 2022
@marrobi marrobi deleted the marrobi/issue2625 branch September 29, 2022 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

With auto_create no interactive user is the owner of the Workspace Enterprise App
2 participants