Skip to content

Commit

Permalink
🐛Do not send "/login" request multiple times on application init (#149)
Browse files Browse the repository at this point in the history
  • Loading branch information
shepilov authored Jul 17, 2023
1 parent 6cbdc9d commit 28ec7c1
Show file tree
Hide file tree
Showing 10 changed files with 149 additions and 111 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export default () => {
centered
closable={false}
title={null}
visible={open}
open={open}
footer={null}
destroyOnClose={true}
width={ModalManager.getPosition()?.size?.width || '700px'}
Expand Down
4 changes: 3 additions & 1 deletion tdrive/frontend/src/app/features/auth/auth-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class AuthService {

const config = InitService.server_infos?.configuration?.accounts[accountType];

console.log(`Use "${Globals.environment.env_dev_auth}" account type for authorization`)
if (Globals.environment.env_dev_auth) accountType = Globals.environment.env_dev_auth;

if (accountType === 'remote') {
Expand Down Expand Up @@ -104,7 +105,6 @@ class AuthService {
onSessionExpired: () => this.onSessionExpired(),
onNewToken: async token => {
this.onNewToken(token);

// TODO: Change the basic auth to return this new token on init
if (this.initState === 'initializing') {
const user = await this.comleteInit();
Expand All @@ -118,6 +118,8 @@ class AuthService {
resolve(null);
},
});
this.logger.info("Init completed")
resolve(null);
});
}

Expand Down
4 changes: 2 additions & 2 deletions tdrive/frontend/src/app/features/auth/jwt-storage-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class JWTStorage {
this.timeDelta = new Date().getTime() / 1000 - jwtData.time;
this.jwtData.expiration += this.timeDelta - 5 * 60; //Force reduce expiration by 5 minutes
this.jwtData.refresh_expiration += this.timeDelta - 5 * 60; //Force reduce expiration by 5 minutes

this.logger.info("Update jwt token in local storage")
LocalStorage.setItem('jwt', this.jwtData);
}
}
Expand Down Expand Up @@ -147,7 +147,7 @@ class JWTStorage {
}

async renew(): Promise<JWTDataType> {
const token = await ConsoleAPIClient.getNewAccessToken();
const token = await ConsoleAPIClient.renewAccessToken();

if (!token) {
throw new Error('Can not get a new access token');
Expand Down
43 changes: 29 additions & 14 deletions tdrive/frontend/src/app/features/auth/login-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,13 @@ import Application from '../applications/services/application-service';
import { UserType } from '@features/users/types/user';
import { Cookies } from 'react-cookie';
import InitService from '../global/services/init-service';
import { useRecoilState } from "recoil";
import { CurrentUserState } from "features/users/state/atoms/current-user";

class Login extends Observable {

private static logInOngoing = false;

// Promise resolved when user is defined
userIsSet!: Promise<string>;
resolveUser!: (userId: string) => void;
Expand Down Expand Up @@ -80,12 +85,14 @@ class Login extends Observable {
}

if (!AuthService.isInitialized()) {
this.logger.log("Auth service is not initialized, init ...")
this.reset();
await AuthService.init();
this.logger.info("Auth service initialized");

const redirectUrl = this.cookies.get('pending-redirect');
if (redirectUrl) {
console.log('Got pending redirect to', redirectUrl);
this.logger.info('Got pending redirect to', redirectUrl);
this.cookies.remove('pending-redirect');
setTimeout(() => {
document.location.href = redirectUrl;
Expand All @@ -104,6 +111,8 @@ class Login extends Observable {
}

async updateUser(callback?: (err: Error | null, user?: UserType) => void): Promise<void> {
this.logger.info("LoginService:: Try to update user info ")

if (Globals.store_public_access_get_data) {
this.firstInit = true;
this.state = 'logged_out';
Expand All @@ -114,7 +123,7 @@ class Login extends Observable {
AuthService.updateUser(async user => {
this.logger.debug('User update result', user);
if (!user) {
if (!this.pingServer()) {
if (!(await this.pingServer())) {
//We are disconnected
console.log('We are disconnected, we will get user again in 10 seconds');
setTimeout(() => {
Expand Down Expand Up @@ -167,27 +176,33 @@ class Login extends Observable {
});
}

login(params: any, hide_load = false) {
if (!hide_load) {
this.login_loading = true;
}
this.login_error = false;
this.notify();
async login(params: any, hide_load = false) {
if (!Login.logInOngoing) {
this.logger.debug("Try to login");
if (!hide_load) {
this.login_loading = true;
}
this.login_error = false;
this.notify();

AuthService.login(params)
.then(async result => {
try {
const result = await AuthService.login(params);
this.login_loading = false;
if (!result) {
this.login_error = true;
this.notify();
return;
}
await this.updateUser();
})
.catch(err => {
} catch (err) {
this.logger.error('Can not login', err);
// TODO display a modal message
});
} finally {
this.logger.debug('Login process finished');
Login.logInOngoing = false;
}
} else {
this.logger.debug("Login is already in process ...");
}
}

async logout(reload = false) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import { getAsFrontUrl } from '@features/global/utils/URLUtils';
import { TdriveService } from '../../../global/framework/registry-decorator-service';
import EnvironmentService from '../../../global/framework/environment-service';
import { AuthProvider, InitParameters } from '../auth-provider';
import ConsoleService from '@features/console/services/console-service';
import jwtStorageService, { JWTDataType } from '@features/auth/jwt-storage-service';
import LocalStorage from '@features/global/framework/local-storage-service';
import ConsoleApiClient from '@features/console/api/console-api-client';
import JwtStorageService from "@features/auth/jwt-storage-service";

const OIDC_CALLBACK_URL = '/oidccallback';
const OIDC_SIGNOUT_URL = '/signout';
Expand Down Expand Up @@ -55,11 +56,11 @@ export default class OIDCAuthProviderService
scope: 'openid profile email address phone offline_access',
post_logout_redirect_uri: getAsFrontUrl(OIDC_SIGNOUT_URL),
silent_redirect_uri: getAsFrontUrl(OIDC_SILENT_URL),
automaticSilentRenew: true,
automaticSilentRenew: false,
loadUserInfo: true,
accessTokenExpiringNotificationTime: 10,
filterProtocolClaims: true,
monitorSession: false,
monitorSession: true,
});

// For logout if signout or logout endpoint called
Expand All @@ -69,6 +70,18 @@ export default class OIDCAuthProviderService
this.signOut();
}

this.userManager.events.addUserSessionChanged((... args) => {
this.logger.debug('User Session changed:', args);
});

this.userManager.events.addSilentRenewError((... args) => {
this.logger.debug('Silent Renew Error:', args);
});

this.userManager.events.addUserUnloaded((... args) => {
this.logger.debug('User unloaded:', args);
});

this.userManager.events.addUserLoaded((user: any, ...args) => {
this.logger.debug('New User Loaded:', user, args);
this.logger.debug('Acess_token: ', user.access_token);
Expand All @@ -83,45 +96,40 @@ export default class OIDCAuthProviderService
await this.userManager?.removeUser();
await this.signIn();
});

this.userManager.events.addSilentRenewError((...args) => {
console.error('Silent Renew Error:', args);
});
}

this.logger.info("Init completed")
return this;
}

async signIn(): Promise<void> {
this.logger.info('Signin');

try {
await this.userManager!.signinRedirectCallback();
await this.userManager?.signinRedirectCallback();
} catch (e) {
console.log('Not connected, connect through SSO');
}

const user = await this.userManager?.getUser();

if (user) {
await this.getJWTFromOidcToken(user, (err, jwt) => {
if (err) {
this.logger.error(
'OIDC user loaded listener, error while getting the JWT from OIDC token',
err,
);
this.signinRedirect();
}

try {
const jwt = await this.getJWTFromOidcToken(user);
if (!this.initialized) {
this.onInitialized();
this.initialized = true;
} else {
jwt && this.params!.onNewToken(jwt);
}
});
this.logger.info("Setting new access token");
await this.params?.onNewToken(jwt);
} catch (err) {
this.logger.error(
'OIDC user loaded listener, error while getting the JWT from OIDC token',
err,
);
await this.signinRedirect();
}
} else {
this.userManager?.signinRedirect();
await this.signinRedirect();
}
}

Expand Down Expand Up @@ -154,28 +162,27 @@ export default class OIDCAuthProviderService
* Try to get a new JWT token from the OIDC one:
* Call the backend with the OIDC token, it will use it to get a new token from console
*/
private async getJWTFromOidcToken(
user: Oidc.User,
callback: (err?: Error, accessToken?: JWTDataType) => void,
): Promise<void> {
private async getJWTFromOidcToken(user: Oidc.User): Promise<JWTDataType> {
if (!user) {
this.logger.info('getJWTFromOidcToken, Cannot getJWTFromOidcToken with a null user');
callback(new Error('Cannot getJWTFromOidcToken with a null user'));
return;
throw new Error('Cannot getJWTFromOidcToken with a null user');
}

if (user.expired) {
// TODO: try to get a new token from refresh one before asking for a JWT token
this.logger.info('getJWTFromOidcToken, user expired');
}

ConsoleService.getNewAccessToken(
const jwt = await ConsoleApiClient.getNewAccessToken(
{ id_token: user.id_token, access_token: user.access_token },
callback,
);

JwtStorageService.updateJWT(jwt)

return jwt;
}

signinRedirect() {
async signinRedirect() {
if (document.location.href.indexOf('/login') === -1) {
//Save requested URL for after redirect / sign-in
LocalStorage.setItem('requested_url', {
Expand All @@ -186,7 +193,9 @@ export default class OIDCAuthProviderService

jwtStorageService.clear();

if (this.userManager) this.userManager.signinRedirect();
if (this.userManager) {
await this.userManager.signinRedirect();
}
}

onInitialized() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import Api from '@features/global/framework/api-service';
import { TdriveService } from '@features/global/framework/registry-decorator-service';
import JWTStorage, { JWTDataType } from '@features/auth/jwt-storage-service';
import Logger from "features/global/framework/logger-service";

type LoginParams = {
email: string;
Expand All @@ -16,8 +17,20 @@ type SignupParams = {
username: string;
};

type AccessTokenResponse = {
statusCode: string;
access_token: JWTDataType;
}

type AccessTokenRequest = {
oidc_id_token: string;
}

@TdriveService('ConsoleAPIClientService')
class ConsoleAPIClient {

logger = Logger.getLogger('ConsoleAPIClient');

login(params: LoginParams, disableJWTAuthentication = false): Promise<string> {
return Api.post<LoginParams, { access_token: string }>(
'/internal/services/console/v1/login',
Expand All @@ -39,7 +52,29 @@ class ConsoleAPIClient {
return res;
}

getNewAccessToken(): Promise<JWTDataType> {
public async getNewAccessToken(
currentToken: { access_token: string; id_token: string }
): Promise<JWTDataType> {
this.logger.debug(
`getNewAccessToken, get new token from current token ${JSON.stringify(currentToken)}`,
);
const response = await Api.post<AccessTokenRequest, AccessTokenResponse>(
'/internal/services/console/v1/login',
{ oidc_id_token: currentToken.id_token });

if (response.statusCode && !response.access_token) {
this.logger.error(
'getNewAccessToken, Can not retrieve access_token from console. Response was',
response,
);
throw new Error('Can not retrieve access_token from console');
}
// the input access_token is potentially expired and so the response contains an error.
// we should be able to refresh the token or renew it in some way...
return response.access_token;
}

renewAccessToken(): Promise<JWTDataType> {
if (JWTStorage.isRefreshExpired() && JWTStorage.isAccessExpired()) {
throw new Error('Can not get access token as both access and refresh token are expired');
}
Expand Down
Loading

0 comments on commit 28ec7c1

Please sign in to comment.