-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Properly handle password change for users authenticated with provider other than basic
.
#55206
Changes from all commits
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 |
---|---|---|
|
@@ -130,7 +130,7 @@ export const security = kibana => | |
); | ||
|
||
server.expose({ | ||
getUser: request => securityPlugin.authc.getCurrentUser(KibanaRequest.from(request)), | ||
getUser: async request => securityPlugin.authc.getCurrentUser(KibanaRequest.from(request)), | ||
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. note: for BWC reasons kept legacy API async. |
||
}); | ||
|
||
initLoginView(securityPlugin, server); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import { AuthenticatedUser } from '../../common/model'; | |
import { AccountManagementPage } from './account_management_page'; | ||
|
||
import { coreMock } from 'src/core/public/mocks'; | ||
import { mockAuthenticatedUser } from '../../common/model/authenticated_user.mock'; | ||
import { securityMock } from '../mocks'; | ||
import { userAPIClientMock } from '../management/users/index.mock'; | ||
|
||
|
@@ -19,11 +20,10 @@ interface Options { | |
realm?: string; | ||
} | ||
const createUser = ({ withFullName = true, withEmail = true, realm = 'native' }: Options = {}) => { | ||
return { | ||
return mockAuthenticatedUser({ | ||
full_name: withFullName ? 'Casey Smith' : '', | ||
username: 'csmith', | ||
email: withEmail ? '[email protected]' : '', | ||
enabled: true, | ||
roles: [], | ||
authentication_realm: { | ||
type: realm, | ||
|
@@ -33,7 +33,7 @@ const createUser = ({ withFullName = true, withEmail = true, realm = 'native' }: | |
type: realm, | ||
name: realm, | ||
}, | ||
}; | ||
}); | ||
}; | ||
|
||
function getSecuritySetupMock({ currentUser }: { currentUser: AuthenticatedUser }) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,13 +80,6 @@ export interface ProviderLoginAttempt { | |
* Login attempt can have any form and defined by the specific provider. | ||
*/ | ||
value: unknown; | ||
|
||
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. note: reducing API surface. This workaround was introduced exactly for this use case and we don't need it anymore. |
||
/** | ||
* Indicates whether login attempt should be performed in a "stateless" manner. If `true` provider | ||
* performing login will neither be able to retrieve or update existing state if any nor persist | ||
* any new state it may produce as a result of the login attempt. It's `false` by default. | ||
*/ | ||
stateless?: boolean; | ||
} | ||
|
||
export interface AuthenticatorOptions { | ||
|
@@ -107,12 +100,12 @@ const providerMap = new Map< | |
providerSpecificOptions?: AuthenticationProviderSpecificOptions | ||
) => BaseAuthenticationProvider | ||
>([ | ||
['basic', BasicAuthenticationProvider], | ||
['kerberos', KerberosAuthenticationProvider], | ||
['saml', SAMLAuthenticationProvider], | ||
['token', TokenAuthenticationProvider], | ||
['oidc', OIDCAuthenticationProvider], | ||
['pki', PKIAuthenticationProvider], | ||
[BasicAuthenticationProvider.type, BasicAuthenticationProvider], | ||
[KerberosAuthenticationProvider.type, KerberosAuthenticationProvider], | ||
[SAMLAuthenticationProvider.type, SAMLAuthenticationProvider], | ||
[TokenAuthenticationProvider.type, TokenAuthenticationProvider], | ||
[OIDCAuthenticationProvider.type, OIDCAuthenticationProvider], | ||
[PKIAuthenticationProvider.type, PKIAuthenticationProvider], | ||
]); | ||
|
||
function assertRequest(request: KibanaRequest) { | ||
|
@@ -254,7 +247,7 @@ export class Authenticator { | |
|
||
// If we detect an existing session that belongs to a different provider than the one requested | ||
// to perform a login we should clear such session. | ||
let existingSession = attempt.stateless ? null : await this.getSessionValue(sessionStorage); | ||
let existingSession = await this.getSessionValue(sessionStorage); | ||
if (existingSession && existingSession.provider !== attempt.provider) { | ||
this.logger.debug( | ||
`Clearing existing session of another ("${existingSession.provider}") provider.` | ||
|
@@ -281,7 +274,7 @@ export class Authenticator { | |
(authenticationResult.failed() && getErrorStatusCode(authenticationResult.error) === 401); | ||
if (existingSession && shouldClearSession) { | ||
sessionStorage.clear(); | ||
} else if (!attempt.stateless && authenticationResult.shouldUpdateState()) { | ||
} else if (authenticationResult.shouldUpdateState()) { | ||
const { idleTimeoutExpiration, lifespanExpiration } = this.calculateExpiry(existingSession); | ||
sessionStorage.set({ | ||
state: authenticationResult.state, | ||
|
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.
note: these changes aren't mandatory, just forgot to remove this cleanup. For the majority of places I tried to not change anything.