-
Notifications
You must be signed in to change notification settings - Fork 944
Add App Check token to Auth requests #6982
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
Changes from all commits
fadca3b
082d181
8ce92e6
b4a9a96
a734a6c
968629d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
--- | ||
'@firebase/auth': minor | ||
'@firebase/auth-compat': minor | ||
--- | ||
|
||
App Check <> Auth integration |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -63,36 +63,38 @@ export function registerAuth(clientPlatform: ClientPlatform): void { | |||
const app = container.getProvider('app').getImmediate()!; | ||||
const heartbeatServiceProvider = | ||||
container.getProvider<'heartbeat'>('heartbeat'); | ||||
const appCheckServiceProvider = | ||||
container.getProvider<'app-check-internal'>('app-check-internal'); | ||||
const { apiKey, authDomain } = app.options; | ||||
return ((app, heartbeatServiceProvider) => { | ||||
_assert( | ||||
apiKey && !apiKey.includes(':'), | ||||
AuthErrorCode.INVALID_API_KEY, | ||||
{ appName: app.name } | ||||
); | ||||
// Auth domain is optional if IdP sign in isn't being used | ||||
_assert(!authDomain?.includes(':'), AuthErrorCode.ARGUMENT_ERROR, { | ||||
appName: app.name | ||||
}); | ||||
const config: ConfigInternal = { | ||||
apiKey, | ||||
authDomain, | ||||
clientPlatform, | ||||
apiHost: DefaultConfig.API_HOST, | ||||
tokenApiHost: DefaultConfig.TOKEN_API_HOST, | ||||
apiScheme: DefaultConfig.API_SCHEME, | ||||
sdkClientVersion: _getClientVersion(clientPlatform) | ||||
}; | ||||
|
||||
const authInstance = new AuthImpl( | ||||
app, | ||||
heartbeatServiceProvider, | ||||
config | ||||
); | ||||
_initializeAuthInstance(authInstance, deps); | ||||
_assert( | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why are we getting rid of the anon function here? I think we copied this from Firestore originally (not exactly sure why it's done this way) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeahh, I wasn't sure why we had the anon function either and didn't notice a difference in behavior with / without it? I think Firestore currently doesn't use an anon function:
Will leave this open if other folks have an opinion about it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 to remove the anon function if we did not observe any behavior change There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool cool |
||||
apiKey && !apiKey.includes(':'), | ||||
AuthErrorCode.INVALID_API_KEY, | ||||
{ appName: app.name } | ||||
); | ||||
// Auth domain is optional if IdP sign in isn't being used | ||||
_assert(!authDomain?.includes(':'), AuthErrorCode.ARGUMENT_ERROR, { | ||||
appName: app.name | ||||
}); | ||||
const config: ConfigInternal = { | ||||
apiKey, | ||||
authDomain, | ||||
clientPlatform, | ||||
apiHost: DefaultConfig.API_HOST, | ||||
tokenApiHost: DefaultConfig.TOKEN_API_HOST, | ||||
apiScheme: DefaultConfig.API_SCHEME, | ||||
sdkClientVersion: _getClientVersion(clientPlatform) | ||||
}; | ||||
|
||||
const authInstance = new AuthImpl( | ||||
app, | ||||
heartbeatServiceProvider, | ||||
appCheckServiceProvider, | ||||
config | ||||
); | ||||
_initializeAuthInstance(authInstance, deps); | ||||
|
||||
return authInstance; | ||||
})(app, heartbeatServiceProvider); | ||||
return authInstance; | ||||
}, | ||||
ComponentType.PUBLIC | ||||
) | ||||
|
Uh oh!
There was an error while loading. Please reload this page.