Skip to content
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

Fixes anonymous auth flow to work with SAML #1817

2 changes: 1 addition & 1 deletion public/apps/login/login-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ export function LoginPage(props: LoginPageDeps) {
</EuiFormRow>
);

if (authOpts.length > 1) {
if (authOpts.length > 0) {
DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved
if (props.config.auth.anonymous_auth_enabled) {
const anonymousConfig = props.config.ui[AuthType.ANONYMOUS].login;
formBody.push(
Expand Down
11 changes: 11 additions & 0 deletions server/auth/types/authentication_type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ export abstract class AuthenticationType implements IAuthenticationType {
const authHeaders = {};
let cookie: SecuritySessionCookie | null | undefined;
let authInfo: any | undefined;

if (this.config.auth.anonymous_auth_enabled) {
const anonymousAuthHeaders = { _auth_request_type_: 'anonymous' };
Object.assign(authHeaders, anonymousAuthHeaders);
}

// if this is an REST API call, suppose the request includes necessary auth header
// see https://www.elastic.co/guide/en/opensearch-dashboards/master/using-api.html
if (this.requestIncludesAuthInfo(request)) {
Expand Down Expand Up @@ -153,10 +159,14 @@ export abstract class AuthenticationType implements IAuthenticationType {
if (request.url.pathname && request.url.pathname.startsWith('/bundles/')) {
return toolkit.notHandled();
}
console.log('Request is unauthorized');
console.log(request.url);
console.log(request.route);

// send to auth workflow
return this.handleUnauthedRequest(request, response, toolkit);
}
console.log('we have a cookie: ' + JSON.stringify(cookie));

// extend session expiration time
if (this.config.session.keepalive) {
Expand Down Expand Up @@ -211,6 +221,7 @@ export abstract class AuthenticationType implements IAuthenticationType {
}
if (!authInfo) {
authInfo = await this.securityClient.authinfo(request, authHeaders);
console.log(authInfo);
}
authState.authInfo = authInfo;

Expand Down
22 changes: 7 additions & 15 deletions server/auth/types/basic/basic_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,21 +111,13 @@ export class BasicAuthentication extends AuthenticationType {
request,
this.coreSetup.http.basePath.serverBasePath
);
if (this.config.auth.anonymous_auth_enabled) {
const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${ANONYMOUS_AUTH_LOGIN}?${nextUrlParam}`;
return response.redirected({
headers: {
location: `${redirectLocation}`,
},
});
} else {
Comment on lines -114 to -121
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if block caused all logout requests to be redirected to the anonymous login page when anonymous auth is enabled. This is incorrect. All logout requests should redirect to login page. (Similar change below for multi_auth.ts)

const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${LOGIN_PAGE_URI}?${nextUrlParam}`;
return response.redirected({
headers: {
location: `${redirectLocation}`,
},
});
}

const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${LOGIN_PAGE_URI}?${nextUrlParam}`;
return response.redirected({
headers: {
location: `${redirectLocation}`,
},
});
} else {
return response.unauthorized({
body: `Authentication required`,
Expand Down
7 changes: 6 additions & 1 deletion server/auth/types/basic/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,9 @@ export class BasicAuthRoutes {
}
context.security_plugin.logger.info('The Redirect Path is ' + redirectUrl);
try {
user = await this.securityClient.authenticateWithHeaders(request, {});
user = await this.securityClient.authenticateWithHeaders(request, {
_auth_request_type_: 'anonymous',
DarshitChanpura marked this conversation as resolved.
Show resolved Hide resolved
});
Copy link
Member Author

@DarshitChanpura DarshitChanpura Mar 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope of this PR, but code between lines 214-228 is dead-code because multi_tenancy is never set upon receiving auth response from backend.

The dead-code:

if (user.multitenancy_enabled) {
const selectTenant = resolveTenant({
request,
username: user.username,
roles: user.roles,
availableTenants: user.tenants,
config: this.config,
cookie: sessionStorage,
multitenancyEnabled: user.multitenancy_enabled,
privateTenantEnabled: user.private_tenant_enabled,
defaultTenant: user.default_tenant,
});
sessionStorage.tenant = selectTenant;
}

this is the reason:

return {
username: esResponse.user_name,
roles: esResponse.roles,
backendRoles: esResponse.backend_roles,
tenants: esResponse.tenants,
selectedTenant: esResponse.user_requested_tenant,
};

as we can see that the property multi_tenancy for user object is never set, hence the if condition is never true.

} catch (error) {
context.security_plugin.logger.error(
`Failed authentication: ${error}. Redirecting to Login Page`
Expand All @@ -200,6 +202,8 @@ export class BasicAuthRoutes {
});
}

console.log('Anon user: ' + JSON.stringify(user));

this.sessionStorageFactory.asScoped(request).clear();
const sessionStorage: SecuritySessionCookie = {
username: user.username,
Expand All @@ -209,6 +213,7 @@ export class BasicAuthRoutes {
};

if (user.multitenancy_enabled) {
request.headers._auth_request_type_ = 'anonymous';
const selectTenant = resolveTenant({
request,
username: user.username,
Expand Down
9 changes: 1 addition & 8 deletions server/auth/types/multiple/multi_auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,20 +166,13 @@ export class MultipleAuthentication extends AuthenticationType {
this.coreSetup.http.basePath.serverBasePath
);

if (this.config.auth.anonymous_auth_enabled) {
const redirectLocation = `${this.coreSetup.http.basePath.serverBasePath}${ANONYMOUS_AUTH_LOGIN}?${nextUrlParam}`;
return response.redirected({
headers: {
location: `${redirectLocation}`,
},
});
}
return response.redirected({
headers: {
location: `${this.coreSetup.http.basePath.serverBasePath}${LOGIN_PAGE_URI}?${nextUrlParam}`,
},
});
} else {
console.log('not a page request');
return response.unauthorized();
}
}
Expand Down
Loading