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 Azure AD Auth Provider #1990
Add Azure AD Auth Provider #1990
Conversation
@@ -41,8 +41,12 @@ export type Options = { | |||
}; | |||
|
|||
const readState = (stateString: string): OAuthState => { | |||
|
|||
// This is hardcoded for Azure (breaks other providers) until workaround is found | |||
const fixedState = stateString.substring(38) |
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 is the hardcoded workaround I've put in to get the Azure provider functional, needs a proper solution.
|
||
const sessionManager = new RefreshingAuthSessionManager({ | ||
connector, | ||
defaultScopes: new Set(['openid', 'offline_access', 'profile', 'email']), |
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.
I have set these to match what is defined in the backend provider.ts
file, but unsure of their use or impact on things as they don't seem to have an affect on passport-azure-ad
behaviour.
import { Config } from '@backstage/config'; | ||
import passport from 'passport'; | ||
|
||
import got from 'got'; |
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.
Used to fetch the user's photo, which is not returned in the token and must be retrieved from the Microsoft Graph API's /me
endpoint.
// If no email for user, fallback to preferred_username | ||
emails: [ | ||
{ | ||
value: rawProfile._json.email || rawProfile._json.preferred_username, |
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.
I put this in primarily as most of my test users don't have email's (no Office 365) and I wanted to be able to login. Might make more sense to ultimately leave this undefined in the event of no email address.
}; | ||
} | ||
|
||
async refresh(refreshToken: string, scope: string): Promise<OAuthResponse> { |
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 currently does not work.
* - https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-permissions-and-consent | ||
* - https://docs.microsoft.com/en-us/graph/permissions-reference | ||
*/ | ||
const scope = 'offline_access profile email User.Read'; |
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.
Scopes for passport-azure-ad
need to be set here.
const useCookieInsteadOfSession = true; | ||
const cookieEncryptionKeys = [ | ||
{ key: '12345678901234567890123456789012', iv: '123456789012' }, | ||
{ key: 'abcdefghijklmnopqrstuvwxyzabcdef', iv: 'abcdefghijkl' }, |
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.
passport-azure-ad
requires express-session
or the use of cookies to be sessionless.
@ContrarianChris Can you let me know how I can get access to the information listed in #1967 please so I can build on what you have for the pipelines plugin I am working on please? You sped up my development a bit faster so a way of accessing that information is critical as well so let me how I can access that information from a plugin e.g. like personal access tokens |
Abandoning this in favour of #2056 |
Hey, I just made a Pull Request!
I've managed to get a semi-working implementation going for Azure AD as an OIDC auth provider but have hit some blockers. Not so much with my understanding of how Backstage auth works, though a few things go a bit beyond me, more with how passport-azure-ad behaves.
Opening this PR as draft to share what I've done so far and hopefully get some help on resolving what is left and finishing things off. I have added a bunch of comments to the changes to add some more info and get some feedback on how I've gone about things.
Progress So Far
Azure option added to Sign In Page...
Azure AD login prompt...
Azure AD consent prompt...
Header message with Backstage ID, using local part of email...
User settings showing Backstage profile display name and picture, plus entry for Azure provider...
Outstanding Issues
Refresh Token Strategy
Hitting refresh attempts to call the
/refresh
endpoint which triggersexecuteRefreshTokenStrategy
, which currently doesn't do anything so you are returned to the Sign In Page and need to auth again.Looking at the Google implementation it looks like
passport-google-oauth20
usespassport-oauth2
under the hood which provides thegetAccessToken
function that handles exchanging the refresh token, but that is as far as I got.There is an open issue for
passport-azure-ad
to support refreshing tokens (AzureAD/passport-azure-ad/issues/297), though it was opened over 3 years ago with no progress as yet. There are a number of threads around the place talking to using MSAL.js on top of Passport.js to handle refreshing tokens but I'm not sure how that would work with Backstage.Hijacked State Parameter
Backstage uses the
state
parameter to encode a nonce value and the environment and then verifies the return of that when handling the auth response.passport-azure-ad
hijacks thestate
parameter and builds its own value which is what is then returned."Custom" state can be provided through a
customState
parameter. Doing this...passport-azure-ad
then sets thestate
parameter to this...Which gives us a return value of...
Which then causes
readState
inOAuthProvider
to throw an invalid state error. We effectively need to.substring(38)
this returned state value to get the original value that Backstage is expecting.This is obviously unique to the Azure provider and the others need the
readState
behaviour to remain unchanged. So the question is how best to handle this?Core-API Implementation
defaultScopes
?What are the
defaultScopes
defined as part ofsessionManager
withinAzureAuth.ts
used for?I ask as they don't seem to have an affect when working with
passport-azure-ad
, as it needs scopes declared in thecreateAzureProvider
function and passed tonew AzureAuthProvider
as part of the options. This seems to be different from the other provider implementations which don't have scopes defined withinprovider.ts
.Wondering whether that is down to my implementation, and/or what fallout there might be that I haven't realised.
Thanks!
✔️ Checklist
yarn test