Skip to content

Commit

Permalink
fix(oauth): refactor oauth services and potential fix for logout issue
Browse files Browse the repository at this point in the history 
The code refactoring also accidentally fixes a potential issue with
OSOAuthService that OSOAuth doesn't unregister fetch interception
upon logout.
  • Loading branch information
@tadayosi
tadayosi committed Mar 11, 2024
1 parent a58ec9a commit 666e342
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 47 deletions.
12 changes: 6 additions & 6 deletions packages/oauth/src/form/form-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,18 @@ interface Headers {
}

export class FormService implements OAuthProtoService {
private userProfile: UserProfile
private readonly login: Promise<boolean>
private formConfig: FormConfig | null
private fetchUnregister: (() => void) | null
private fetchUnregister?: () => void

constructor(formConfig: FormConfig | null, userProfile: UserProfile) {
constructor(
formConfig: FormConfig | null,
private readonly userProfile: UserProfile,
) {
log.debug('Initialising Form Auth Service')
this.userProfile = userProfile
this.userProfile.setOAuthType(FORM_AUTH_PROTOCOL_MODULE)
this.formConfig = formConfig
this.login = this.createLogin()
this.fetchUnregister = null
}

private async createLogin(): Promise<boolean> {
Expand Down Expand Up @@ -171,7 +171,7 @@ export class FormService implements OAuthProtoService {
}

private doLogout(): void {
if (this.fetchUnregister) this.fetchUnregister()
this.fetchUnregister?.()

const currentURI = new URL(window.location.href)
this.clearTokenStorage()
Expand Down
10 changes: 5 additions & 5 deletions packages/oauth/src/oauth-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,17 @@ class OAuthService {
this.userProfile.setError(new Error('Cannot find the osconsole configuration'))
return null
}
log.debug('OAuth config to be processed: ', config)
log.debug('OAuth config to be processed:', config)

log.debug('Adding master uri to profile')
this.userProfile.setMasterUri(relToAbsUrl(config.master_uri || '/master'))
this.userProfile.setMasterKind(config.master_kind || KUBERNETES_MASTER_KIND)
this.userProfile.setMasterUri(relToAbsUrl(config.master_uri ?? '/master'))
this.userProfile.setMasterKind(config.master_kind ?? KUBERNETES_MASTER_KIND)

log.debug('Adding hawtio-mode to profile metadata')
const hawtioMode = config.hawtio?.mode || DEFAULT_HAWTIO_MODE
const hawtioMode = config.hawtio?.mode ?? DEFAULT_HAWTIO_MODE
this.userProfile.addMetadata(HAWTIO_MODE_KEY, hawtioMode)
if (hawtioMode !== DEFAULT_HAWTIO_MODE)
this.userProfile.addMetadata(HAWTIO_NAMESPACE_KEY, config.hawtio?.namespace || DEFAULT_HAWTIO_NAMESPACE)
this.userProfile.addMetadata(HAWTIO_NAMESPACE_KEY, config.hawtio?.namespace ?? DEFAULT_HAWTIO_NAMESPACE)

let protoService: OAuthProtoService | null = null
if (config.form) {
Expand Down
69 changes: 33 additions & 36 deletions packages/oauth/src/openshift/osoauth-service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,26 +45,22 @@ export class OSOAuthService implements OAuthProtoService {
private keepaliveInterval = 10
private keepAliveHandler: NodeJS.Timeout | null = null

private userProfile: UserProfile
private readonly adaptedConfig: Promise<OpenShiftOAuthConfig | null>
private readonly login: Promise<boolean>
private fetchUnregister: (() => void) | null
private fetchUnregister?: () => void

constructor(openShiftConfig: OpenShiftOAuthConfig, userProfile: UserProfile) {
log.debug('Initialising Openshift OAuth Service')
this.userProfile = userProfile
constructor(
openShiftConfig: OpenShiftOAuthConfig,
private readonly userProfile: UserProfile,
) {
log.debug('Initialising OpenShift OAuth Service')
this.userProfile.setOAuthType(OAUTH_OS_PROTOCOL_MODULE)
this.adaptedConfig = this.processConfig(openShiftConfig)
this.login = this.createLogin()
this.fetchUnregister = null
}

private async processConfig(config: OpenShiftOAuthConfig): Promise<OpenShiftOAuthConfig | null> {
if (!config) {
this.userProfile.setError(new Error('Cannot find the openshift auth configuration'))
return null
}
log.debug('OS OAuth config to be processed: ', config)
log.debug('OpenShift OAuth config to be processed:', config)

if (config.oauth_authorize_uri) return config

Expand All @@ -75,8 +71,8 @@ export class OSOAuthService implements OAuthProtoService {
}

// See if web_console_url has been added to config
if (config.web_console_url && config.web_console_url.length > 0) {
log.debug(`Adding web console URI to user profile ${config.web_console_url}`)
if (!isBlank(config.web_console_url)) {
log.debug('Adding web console URI to user profile:', config.web_console_url)
this.userProfile.addMetadata(CLUSTER_CONSOLE_KEY, config.web_console_url)
}

Expand All @@ -89,7 +85,7 @@ export class OSOAuthService implements OAuthProtoService {
config.issuer = metadata.issuer

if (isBlank(config.oauth_authorize_uri) || isBlank(config.oauth_client_id)) {
this.userProfile.setError(new Error('Invalid openshift auth config'))
this.userProfile.setError(new Error('Invalid OpenShift auth config'))
return null
}

Expand All @@ -98,29 +94,29 @@ export class OSOAuthService implements OAuthProtoService {
return config
},
error: err => {
const e: Error = new Error('Failed to contact the oauth metadata uri', { cause: err })
const e = new Error('Failed to contact the oauth metadata uri', { cause: err })
this.userProfile.setError(e)
return null
},
})
}

private setupFetch(config: OpenShiftOAuthConfig) {
if (!config || this.userProfile.hasError()) {
if (this.userProfile.hasError()) {
return
}

log.debug('Intercept Fetch API to attach Openshift auth token to authorization header')
const unregister = fetchIntercept.register({
request: (url, config) => {
log.debug('Intercept Fetch API to attach OpenShift auth token to authorization header')
this.fetchUnregister = fetchIntercept.register({
request: (url, requestConfig) => {
log.debug('Fetch intercepted for oAuth authentication')

if (tokenHasExpired(this.userProfile)) {
const reason = `Cannot navigate to ${url} as token expired so need to logout`
log.debug(reason)

// Unregister this fetch handler before logging out
unregister()
this.fetchUnregister?.()

this.doLogout(config)
}
Expand All @@ -139,20 +135,20 @@ export class OSOAuthService implements OAuthProtoService {
}
}

return [url, { headers, ...config }]
return [url, { headers, ...requestConfig }]
},
})
}

private setupJQueryAjax(config: OpenShiftOAuthConfig) {
if (!config || this.userProfile.hasError()) {
if (this.userProfile.hasError()) {
return
}

log.debug('Set authorization header to Openshift auth token for AJAX requests')
const beforeSend = (xhr: JQueryXHR, settings: JQueryAjaxSettings) => {
if (tokenHasExpired(this.userProfile)) {
log.debug(`Cannot navigate to ${settings.url} as token expired so need to logout`)
log.debug('Cannot navigate to', settings.url, 'as token expired so need to logout')
this.doLogout(config)
return
}
Expand Down Expand Up @@ -237,7 +233,7 @@ export class OSOAuthService implements OAuthProtoService {
}

if (this.userProfile.hasError()) {
log.debug('Cannot login as user profile has an error: ', this.userProfile.getError())
log.debug('Cannot login as user profile has an error:', this.userProfile.getError())
return false
}

Expand All @@ -254,18 +250,18 @@ export class OSOAuthService implements OAuthProtoService {
}

log.debug('Populating user profile with token metadata')
/* Populate the profile with the new token */
this.userProfile.addMetadata<number>(EXPIRES_IN_KEY, tokenParams.expires_in || 0)
this.userProfile.addMetadata<string>(TOKEN_TYPE_KEY, tokenParams.token_type || '')
this.userProfile.addMetadata<number>(OBTAINED_AT_KEY, tokenParams.obtainedAt || 0)
// Populate the profile with the new token
this.userProfile.addMetadata<number>(EXPIRES_IN_KEY, tokenParams.expires_in ?? 0)
this.userProfile.addMetadata<string>(TOKEN_TYPE_KEY, tokenParams.token_type ?? '')
this.userProfile.addMetadata<number>(OBTAINED_AT_KEY, tokenParams.obtainedAt ?? 0)

this.userProfile.setToken(tokenParams.access_token || '')
this.userProfile.setToken(tokenParams.access_token ?? '')

if (this.checkTokenExpired(config)) return false

/* Promote the hawtio mode to expose to third-parties */
// Promote the hawtio mode to expose to third-parties
log.debug('Adding cluster version to profile metadata')
this.userProfile.addMetadata<string>(CLUSTER_VERSION_KEY, config.cluster_version || DEFAULT_CLUSTER_VERSION)
this.userProfile.addMetadata<string>(CLUSTER_VERSION_KEY, config.cluster_version ?? DEFAULT_CLUSTER_VERSION)

// Need fetch for keepalive
this.setupFetch(config)
Expand All @@ -274,7 +270,8 @@ export class OSOAuthService implements OAuthProtoService {
this.setupKeepAlive(config)
return true
} catch (error) {
this.userProfile.setError(error instanceof Error ? error : new Error('Error from checking token'))
const e = error instanceof Error ? error : new Error('Error from checking token')
this.userProfile.setError(e)
return false
}
}
Expand All @@ -285,7 +282,7 @@ export class OSOAuthService implements OAuthProtoService {
}

private doLogout(config: OpenShiftOAuthConfig): void {
if (this.fetchUnregister) this.fetchUnregister()
this.fetchUnregister?.()

const currentURI = new URL(window.location.href)
// The following request returns 403 when delegated authentication with an
Expand Down Expand Up @@ -320,11 +317,11 @@ export class OSOAuthService implements OAuthProtoService {
})

let username = this.userProfile.getToken() // default
if (userInfo && userInfo.metadata?.name) {
username = userInfo.metadata?.name
if (userInfo?.metadata?.name) {
username = userInfo.metadata.name
}

resolve({ username: username, isLogin: true })
resolve({ username, isLogin: true })
userService.setToken(this.userProfile.getToken())
}

Expand Down

0 comments on commit 666e342

Please sign in to comment.