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

Guidance on Invoke-D365LcsApiRefreshToken usage #736

Open
smholvoet opened this issue May 26, 2023 · 10 comments
Open

Guidance on Invoke-D365LcsApiRefreshToken usage #736

smholvoet opened this issue May 26, 2023 · 10 comments

Comments

@smholvoet
Copy link
Contributor

smholvoet commented May 26, 2023

Hi 👋

I'm using this module to apply quality updates to our (rather extensive collection of 🥴) cloud-hosted environments. This works great but I'm not sure how I should make use of the Invoke-D365LcsApiRefreshToken cmdlet.

Pseudo code of the steps we're going through:

1. authenticate via Get-D365LcsApiToken | Set-D365LcsApiConfig, this gets me a valid token for 1 hour
2. fetch details of the target environment, as we need the environment ID later on
3. fetch details of the LCS asset which we're going to deploy, as we need the asset ID later on
4. start the VM
5. meat and bones of the script:
 a. start the deployment via Invoke-D365LcsDeployment
 b. poll the deployment status every 5 minutes, timeout after 6 hours
 c. check for the deployment outcome* ⚠
6. stop the VM* ⚠

* these steps never get executed as I'm no longer authenticated against the LCS API

Steps 1 through 5a work fine, but I'm running into issues as the token expires after 60 min, which is right around the time when the deployment has started and I'm in the process of polling the deployment status every 5 minutes:

Write-Host "Successfully started deployment of asset '$($AssetName)' to environment '$($TargetEnvironmentName)' with activity id '$($DeploymentOperation.ActivityId)'"
Write-Host "Waiting for deployment to complete..."
$DeploymentStatus = Get-D365LcsDeploymentStatus -ActivityId $DeploymentOperation.ActivityId -EnvironmentId $TargetEnvironment.EnvironmentId -SleepInSeconds 0

# Poll deployment status every 5 min, timeout after 6 hours
while ( ($DeploymentStatus -ne "Completed") -and ($DeploymentPollingCount -lt 72) ) {
    $DeploymentStatus = Get-D365LcsDeploymentStatus -ActivityId $DeploymentOperation.ActivityId -EnvironmentId $TargetEnvironment.EnvironmentId -SleepInSeconds 300
    Write-Host "Status: $($DeploymentStatus.OperationStatus)"
    $DeploymentPollingCount++
} 

What would be the best way to incorporate the Invoke-D365LcsApiRefreshToken cmdlet in my script? Adding it to the above while loop would mean that it would refresh every 5 min which I guess would be overkill?

@FH-Inway
Copy link
Member

Hi,

nice use case.

If you don't want to call Invoke-D365LcsApiRefreshToken after a fixed period of time, you can use the expires_on information from the OAuth call. Get-D365LcsApiToken will provide this information (which is a Unix timestamp) in ActiveTokenExpiresOn. Set-D365LcsApiConfig and Get-D365LcsApiConfig also have this information.

You can then use this information to check if the token expires soon and if so, refresh it.

.PARAMETER ActiveTokenExpiresOn
The point in time where the current bearer token will expire
The time is measured in Unix Time, total seconds since 1970-01-01

@smholvoet
Copy link
Contributor Author

Hi, thanks for the reply.

So I could fetch the exact timestamp via Get-D365LcsApiConfig right after initial authentication. Armed with said timestamp, my client id and refresh token I could then add this to my while loop which checks for the deployment's progress:

while ( ($DeploymentStatus -ne "Completed") -and ($DeploymentPollingCount -lt 72) ) {
    if (<check if the token will expire in the next couple of minutes>) { 
        Get-D365LcsApiConfig | Invoke-D365LcsApiRefreshToken | Set-D365LcsApiConfig
    }

    $DeploymentStatus = Get-D365LcsDeploymentStatus -ActivityId $DeploymentOperation.ActivityId -EnvironmentId $TargetEnvironment.EnvironmentId -SleepInSeconds 300
    Write-Host "Status: $($DeploymentStatus.OperationStatus)"
    $DeploymentPollingCount++
}

I'll update this issue if I eventually get this working.

@Splaxi
Copy link
Collaborator

Splaxi commented May 30, 2023

Personally I'd just go for the Refresh on every X minutes. Moving forward faster, keeping stuff simple for other to take over. Looking at how many request the Azure AD is handling during a day, my argument is that getting a refresh token every 5. minutes, doesn't do much harm.

Happy to learn that people at using some of the LCS specific stuff from the module 🔥

@smholvoet
Copy link
Contributor Author

smholvoet commented May 30, 2023

Having some issues with piping the different cmdlets, which I saw here.

Get-D365LcsApiConfig | Invoke-D365LcsApiRefreshToken -Verbose | Set-D365LcsApiConfig

VERBOSE: [10:20:52][Invoke-Authorization] Authenticating against Azure Active Directory (AAD).
VERBOSE: HTTP/1.1 POST with 836-byte payload
VERBOSE: received 469-byte response of content type application/json
[10:20:53][Invoke-Authorization] Something went wrong while working against Azure Active Directory (AAD) | Response status code does not 
indicate success: 400 (Bad Request).
WARNING: [10:20:53][Invoke-Authorization] Stopping because of errors

This however seems to work and generates a fresh access token:

Invoke-D365LcsApiRefreshToken -ClientId $LCS_ClientId -RefreshToken $(Get-D365LcsApiConfig).refreshToken

💡 EDIT: just stumbled upon #496

I needed to explicitly specify the client ID when storing the LCS API config:

Get-D365LcsApiToken -ClientId $LCS_ClientId `
                    -Username $LCS_Username `
                    -Password $LCS_Password `
                    -LcsApiUri $LCS_ApiUri | Set-D365LcsApiConfig -ProjectId $LCS_ProjectId -ClientId $LCS_ClientId 👈

Get-D365LcsApiConfig | Invoke-D365LcsApiRefreshToken | Set-D365LcsApiConfig

@FH-Inway
Copy link
Member

I had the same issue as well in the past. It would be more intuitive if, when piping Get-D365LcsApiToken with the -ClientId parameter to Set-D365LcsApiConfig, the -ClientId parameter would be piped as well.

@Splaxi
Copy link
Collaborator

Splaxi commented May 31, 2023

I hear what you are saying, but from a pure powershell point of view - it is only the real output that is being piped from one cmdlet to another.

-PassThru would be a way to go, as this indicates that more than just the ordinary output will be piped.

But if we were to reorder things a bit, we could show things to following way:

Set-D365LcsApiConfig -ProjectId $LCS_ProjectId -ClientId $LCS_ClientId
Get-D365LcsApiToken -ClientId $LCS_ClientId `
                    -Username $LCS_Username `
                    -Password $LCS_Password `
                    -LcsApiUri $LCS_ApiUri | Set-D365LcsApiConfig

Get-D365LcsApiConfig | Invoke-D365LcsApiRefreshToken | Set-D365LcsApiConfig

Above is just pseudo code - but would that make more sense?

@FH-Inway
Copy link
Member

FH-Inway commented Jun 1, 2023

Hm, maybe if we could pipe Set-D365LcsApiConfig to Get-D365LcsApiToken. Will give that a try when I get the chance.

@FH-Inway
Copy link
Member

Ok, so here is what I was able to figure out:

  • The pseudo code works, you can even remove the -ClientId parameter from the Get-D365LcsApiToken call, because that value is correctly taken from the $Script context.
  • In theory, you could also remove the -LcsApiUri parameter. However, the name of the $Script value for that is incorrect in the Get-D365LcsApiToken cmdlet.
  • You mentioned -PassThru, which as far as I understand is a PowerShell convention to signal to a cmdlet that usually does not return any output to do so. In our case, that would be Set-D365LcsApiConfig, which currently has no output. Instead, the LCS API config values are stored as configuration values (provided by the PSFramework module) and as variables of the $Script context.
  • Most cmdlets make use of the stored LCS API config values. However, Invoke-D365LcsApiRefereshToken does not and instead requires piping or explicit setting of the parameters.

I propose that

  1. The incorrect $Script context variable name for the LCS API URI is fixed (see pr 🐛 fix the name of the LCS API URI config value #737)
  2. 'Invoke-D365LcsApiRefereshToken' is modified to take the LCS API config values from the $Script context.
  3. other LCS cmdlets are checked on how they get their LCS API config values

This would result in the following usage of the token cmdlets:

Set-D365LcsApiConfig -ProjectId $LCS_ProjectId -ClientId $LCS_ClientId -LcsApiUri $LCS_ApiUri
Get-D365LcsApiToken -Username $LCS_Username -Password $LCS_Password | Set-D365LcsApiConfig
Invoke-D365LcsApiRefreshToken | Set-D365LcsApiConfig

Personally, I would prefer an alternative where only the Set|Get-D365LcsApiConfig cmdlets access the $Script context variables and all other LCS cmdlets would either use the output of Get-D365LcsApiConfig as input or have the config parameters explicitly set. That just feels more expressive to me and more clearly communicates what is happening and that the config needs to be set first. But it would also break a lot of workflows, so not an option.

Set-D365LcsApiConfig -ProjectId $LCS_ProjectId -ClientId $LCS_ClientId -LcsApiUri $LCS_ApiUri
Get-D365LcsApiConfig | Get-D365LcsApiToken -Username $LCS_Username -Password $LCS_Password | Set-D365LcsApiConfig
Get-D365LcsApiConfig | Invoke-D365LcsApiRefreshToken | Set-D365LcsApiConfig

@Splaxi
Copy link
Collaborator

Splaxi commented Jun 12, 2023

Thanks for putting in the effort to think about these things and providing feedback, AND a PR with fixes to a hidden bug 🔥

So what do we need to agree on, or work on, after this PR has been merged? Do we need to refactor the Invoke-D365LcsApiRefereshToken ?

@FH-Inway
Copy link
Member

@Splaxi Thanks for the speedy merge :)

Yes, if you agree with the proposal that Invoke-D365LcsApiRefreshToken should take the LCS API config values from the $Script context, I will create a pr for that.

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

No branches or pull requests

3 participants