Skip to content

Managed Identity - improved #92

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

Open
wants to merge 29 commits into
base: main
Choose a base branch
from
Open

Conversation

LuiseFreese
Copy link

tried to take into account your requested changes.

LuiseFreese and others added 8 commits August 12, 2021 16:01
@marcduiker
Copy link
Owner

@LuiseFreese can you update the Link Checker workflow to add this https://graph.microsoft.com.* to the arguments to exclude the graph.microsoft.com links to be checked?

args: --verbose --no-progress --exclude-mail --exclude-loopback --exclude "https?://localhost.*" "https://sandbox.api.sap.com.*" "https://azure-university-app-config.*" "https://192.168.7.108.*" -- "**/*.md"

added as requested `https://graph.microsoft.com.*` to be excluded from checks
@marcduiker
Copy link
Owner

@LuiseFreese can you update the Link Checker workflow to add this https://graph.microsoft.com.* to the arguments to exclude the graph.microsoft.com links to be checked?

args: --verbose --no-progress --exclude-mail --exclude-loopback --exclude "https?://localhost.*" "https://sandbox.api.sap.com.*" "https://azure-university-app-config.*" "https://192.168.7.108.*" -- "**/*.md"

Needs to be in quotes in the yaml so "https://graph.microsoft.com.*" 😅

added `"` round the URL so that YAML likes it.
Copy link
Owner

@marcduiker marcduiker left a comment

Choose a reason for hiding this comment

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

Added mostly some formatting and style changes. Still have to review exercise 4. Could you also add some lines like: Use the PowerShell terminal and type: followed by the instructions you already have. Because to novice users it might not be clear if az cli or PowerShell code should go in the function or in a local terminal.


#Set values
$webAppName="LuiseDemo-functionapp$rand"
$principalId=$(az resource list -n $webAppName --query [*].identity.principalId --out tsv)
Copy link
Owner

Choose a reason for hiding this comment

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

I'm a PowerShell n00b. I see two usages of assigning variables here. Lines 152 and 153 are wrapping the output of the az commands in a $( ... ). This is not used when assigning the variables for graphId and appRoleId. Why this difference? Is it due to the tsv output formatting?

Copy link
Author

Choose a reason for hiding this comment

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

PrincipalId is the Object ID of the system-assigned Managed Identity to which we want to assign the app role.
image and we get this with $principalId = $(az resource list -n $webAppName --query [*].identity.principalId --out tsv) while $webappName is the name of our function app. We need the PrincipalId in the body of the REST call:

$body = "{'principalId':'$principalId','resourceId':'$graphResourceId','appRoleId':'$appRoleId'}"

graphID is the application ID of Microsoft Graph API exposed on AAD, its value is 00000003-0000-0000-c000-000000000000. We get this with $graphId = az ad sp list --query "[?appDisplayName=='Microsoft Graph'].appId | [0]" --all and we need it to get the appRoleId.

The appRoleId is the particular scope, like Group.Read or Teams.Create.

Does this make sense to you?


```powershell

#Get Graph Api service provider (that's later needed for --api)
Copy link
Owner

Choose a reason for hiding this comment

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

I don't understand this part: (that's later needed for --api)

Copy link
Author

Choose a reason for hiding this comment

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

I can delete this... it was more a comment for myself... the syntax for adding permissions is

az ad app permission add --id <ID goes here> --api <api like Graph goes here> --api-permissions <permission scope like Group.Read goes here>=Scope
Does that make sense to you?

LuiseFreese and others added 17 commits August 19, 2021 09:41
LuiseFreese and others added 2 commits August 19, 2021 09:48
adds some "use the PowerShell terminal" to clarify :-)
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.

2 participants