-
Notifications
You must be signed in to change notification settings - Fork 130
Connect-PowerBIServiceAccount provide _-Token_ parameter #427
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
base: master
Are you sure you want to change the base?
Connect-PowerBIServiceAccount provide _-Token_ parameter #427
Conversation
|
@microsoft-github-policy-service agree company="Microsoft" |
| <PackageVersion Include="Microsoft.Identity.Client.Broker" Version="4.64.0" /> | ||
| <PackageVersion Include="Microsoft.Identity.Client.NativeInterop" Version="0.16.2" /> | ||
| <PackageVersion Include="Microsoft.PowerBI.Api" Version="2.14.0" /> | ||
| <PackageVersion Include="Microsoft.IdentityModel.JsonWebTokens" Version="8.15.0" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validate that it didn't break usage of the cmdlet on older systems.
| case PowerBIProfileType.Certificate: | ||
| return await this.Authenticate(profile.UserName, profile.Thumbprint, profile.Environment, logger, settings); | ||
| case PowerBIProfileType.BringYourOwnToken: | ||
| return new PowerBIAccessToken { AccessToken = profile.AccessToken, }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it enough for customer? On the other hand, we return what has been passed in.
| [-DiscoveryUrl <String>] [<CommonParameters>] | ||
| ``` | ||
|
|
||
| ### BringYourOwnToken |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file has to be reviewed by a native speaker :)
Minimum allowed line rate is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Adds a new authentication option to Connect-PowerBIServiceAccount that allows callers to supply their own JWT access token, propagating it through the profile/authentication flow so subsequent API calls can use it.
Changes:
- Introduces a new
BringYourOwnTokenparameter set with-TokenonConnect-PowerBIServiceAccount, including JWT lifetime validation. - Extends profile/authentication abstractions (
IPowerBIProfile,PowerBIProfileType,AuthenticationFactorySelector) to support a BYOT login type. - Updates help documentation and adds unit tests for token-format/expiry validation and parameter-set binding behavior.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Modules/Profile/Commands.Profile/help/Connect-PowerBIServiceAccount.md | Documents the new -Token parameter set and adds an example. |
| src/Modules/Profile/Commands.Profile/GetPowerBIAccessToken.cs | Minor whitespace cleanup. |
| src/Modules/Profile/Commands.Profile/ConnectPowerBIServiceAccount.cs | Adds BYOT parameter set and JWT validation prior to creating a profile. |
| src/Modules/Profile/Commands.Profile.Test/ConnectPowerBIServiceAccountTests.cs | Adds tests covering malformed/expired/valid tokens and invalid parameter combinations. |
| src/Common/Common.Abstractions/PowerBIProfileType.cs | Adds BringYourOwnToken enum value. |
| src/Common/Common.Abstractions/PowerBIProfile.cs | Adds AccessToken storage and a BYOT constructor. |
| src/Common/Common.Abstractions/Interfaces/IPowerBIProfile.cs | Adds AccessToken to the profile interface. |
| src/Common/Commands.Common/Commands.Common.csproj | Adds Microsoft.IdentityModel.JsonWebTokens dependency. |
| src/Common/Commands.Common/AuthenticationFactorySelector.cs | Adds BYOT handling in Authenticate(profile, ...). |
| src/Common/Commands.Common.Test/TestProfile.cs | Implements new AccessToken property required by IPowerBIProfile. |
| Directory.Packages.props | Centrally pins Microsoft.IdentityModel.JsonWebTokens version. |
| .editorconfig | Enables trimming trailing whitespace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| case PowerBIProfileType.Certificate: | ||
| return await this.Authenticate(profile.UserName, profile.Thumbprint, profile.Environment, logger, settings); | ||
| case PowerBIProfileType.BringYourOwnToken: | ||
| return new PowerBIAccessToken { AccessToken = profile.AccessToken, }; |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For PowerBIProfileType.BringYourOwnToken, the returned PowerBIAccessToken does not populate AuthorizationHeader. This breaks Get-PowerBIAccessToken, which outputs token.AuthorizationHeader and will return null for BYOT profiles. Populate AuthorizationHeader (and ideally AccessTokenType) when constructing the token.
| return new PowerBIAccessToken { AccessToken = profile.AccessToken, }; | |
| return new PowerBIAccessToken | |
| { | |
| AccessToken = profile.AccessToken, | |
| AuthorizationHeader = $"Bearer {profile.AccessToken}", | |
| AccessTokenType = "Bearer", | |
| }; |
| using Microsoft.PowerBI.Common.Abstractions; | ||
| using Microsoft.PowerBI.Common.Abstractions.Interfaces; | ||
| using Microsoft.PowerBI.Common.Authentication; | ||
| using Microsoft.IdentityModel.JsonWebTokens; |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using Microsoft.IdentityModel.JsonWebTokens; is unused in this file (no references), which can generate compiler warnings. Remove the unused using.
| using Microsoft.IdentityModel.JsonWebTokens; |
| The certificate must be installed in either CurrentUser or LocalMachine certificate store (LocalMachine requires administrator access) with a private key installed. | ||
|
|
||
|
|
||
| Use provided _Token_ for authentication during api calls. |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This documentation sentence should be clearer and more consistent: use “API” (not “api”), and consider explicitly warning that the token is a secret and should not be pasted into logs/history. Also clarify whether -Token expects the raw JWT or an Authorization header value (e.g., with/without the Bearer prefix).
| Use provided _Token_ for authentication during api calls. | |
| Use the provided _Token_ (a raw OAuth 2.0 access token/JWT value, without the `Bearer ` prefix) for authentication when calling the Power BI REST API. Treat this token as a secret and do not paste it into logs, scripts, or command-line history. |
|
|
||
| ### Example 5 | ||
| ```powershell | ||
| PS C:\> Connect-PowerBIServiceAccount -Token eyJhbGciOiJIUzI1NiJ9.eyJ0ZXN0X2NsYWltIjp0cnVlLCJpc3MiOiJ1cm46ZXhhbXBsZTppc3N1ZXIiLCJhdWQiOiJ1cm46ZXhhbXBsZTphdWRpZW5jZSIsImV4cCI6MjcxNzQ0Njc4NSwiaWF0IjoxNzcwNzE4Nzg1fQ.nOXgwieIpeFB9Svxxt6Z4_RkWVWSiVJcxbBzlPTaQJQ |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example includes a full JWT-like token inline. Even if it’s a dummy token, it’s safer to either clearly label it as fake and/or truncate/redact it to discourage copying secrets into command history.
| PS C:\> Connect-PowerBIServiceAccount -Token eyJhbGciOiJIUzI1NiJ9.eyJ0ZXN0X2NsYWltIjp0cnVlLCJpc3MiOiJ1cm46ZXhhbXBsZTppc3N1ZXIiLCJhdWQiOiJ1cm46ZXhhbXBsZTphdWRpZW5jZSIsImV4cCI6MjcxNzQ0Njc4NSwiaWF0IjoxNzcwNzE4Nzg1fQ.nOXgwieIpeFB9Svxxt6Z4_RkWVWSiVJcxbBzlPTaQJQ | |
| PS C:\> Connect-PowerBIServiceAccount -Token eyJhbGciOiJIUzI1NiJ9.fake_payload_and_signature_redacted_for_example |
| using Microsoft.PowerBI.Commands.Common; | ||
| using Microsoft.PowerBI.Common.Abstractions; | ||
| using Microsoft.PowerBI.Common.Abstractions.Interfaces; | ||
| using Microsoft.PowerBI.Common.Authentication; |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using Microsoft.PowerBI.Common.Authentication; appears unused in this file and may trigger build warnings (and potentially fail builds if warnings are treated as errors). Remove the unused using.
| using Microsoft.PowerBI.Common.Authentication; |
| <PackageReference Include="System.Runtime.Serialization.Json" /> | ||
| <PackageReference Include="PowerShellStandard.Library" /> | ||
| <PackageReference Include="System.Runtime.Serialization.Json" /> | ||
| <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" /> |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Microsoft.IdentityModel.JsonWebTokens is referenced from Commands.Common.csproj, but the actual usage introduced in this PR is in the Profile module (ConnectPowerBIServiceAccount). Consider moving this PackageReference to Commands.Profile.csproj to avoid adding an otherwise-unused dependency to Commands.Common.
| <PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" /> |
| ``` | ||
|
|
||
| ### -Token | ||
| Jwt token that will be attached as authentication header for API calls. |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parameter description uses “Jwt” and could be more precise. Consider using “JWT” and specify exactly how it’s applied (e.g., sent as Authorization: Bearer <token>), and whether callers should pass only the token value (not the full header).
| Jwt token that will be attached as authentication header for API calls. | |
| JWT access token to use for authentication; pass only the token value, which will be sent as the `Authorization: Bearer <token>` header for API calls. |
| } | ||
| else | ||
| { | ||
| this.Logger.WriteError("Token validation failed"); |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the BringYourOwnToken flow, if token validation fails but result.Exception is null, this only writes a non-terminating error and continues, which can leave an invalid token stored in the profile. Make this a terminating failure (e.g., throw a SecurityTokenValidationException or call ThrowTerminatingError) so the cmdlet cannot succeed with an invalid token.
| this.Logger.WriteError("Token validation failed"); | |
| throw new SecurityTokenValidationException("Token validation failed"); |
| ValidateIssuer = false, | ||
| ValidateAudience = false, | ||
| ValidateLifetime = true, | ||
| SignatureValidator = (token, parameters) => new JsonWebToken(token), |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SignatureValidator = (token, parameters) => ... lambda parameter name shadows the outer token method parameter, which is confusing. Rename the lambda parameter (e.g., rawToken) to avoid shadowing and improve readability.
| SignatureValidator = (token, parameters) => new JsonWebToken(token), | |
| SignatureValidator = (rawToken, parameters) => new JsonWebToken(rawToken), |
| ps.AddParameter(nameof(ConnectPowerBIServiceAccount.Credential), new PSCredential("password", new SecureString())); | ||
|
|
||
| // Act & Assert | ||
| Assert.ThrowsException<ParameterBindingException>(ps.Invoke); |
Copilot
AI
Feb 12, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disposable 'SecureString' is created but not disposed.
| ps.AddParameter(nameof(ConnectPowerBIServiceAccount.Credential), new PSCredential("password", new SecureString())); | |
| // Act & Assert | |
| Assert.ThrowsException<ParameterBindingException>(ps.Invoke); | |
| using (var securePassword = new SecureString()) | |
| { | |
| var credential = new PSCredential("password", securePassword); | |
| ps.AddParameter(nameof(ConnectPowerBIServiceAccount.Credential), credential); | |
| // Act & Assert | |
| Assert.ThrowsException<ParameterBindingException>(ps.Invoke); | |
| } |
Provide an option to bring your own token for authentication