Skip to content

Follow up 944: authentication sessions are not persistent #1003

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,14 @@ export class AuthenticationClientService

readonly onSessionDidChange = this.onSessionDidChangeEmitter.event;

onStart(): void {
async onStart(): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to make it async, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, correct, that was left in from 0a7dbcc

this.toDispose.push(this.onSessionDidChangeEmitter);
this.service.setClient(this);
this.service
.session()
.then((session) => this.notifySessionDidChange(session));

this.setOptions();
this.service.initAuthSession()
this.setOptions().then(() => this.service.initAuthSession());

this.arduinoPreferences.onPreferenceChanged((event) => {
if (event.preferenceName.startsWith('arduino.auth.')) {
Expand All @@ -60,8 +59,8 @@ export class AuthenticationClientService
});
}

setOptions(): void {
this.service.setOptions({
setOptions(): Promise<void> {
return this.service.setOptions({
redirectUri: `http://localhost:${serverPort}/callback`,
responseType: 'code',
clientID: this.arduinoPreferences['arduino.auth.clientID'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export interface AuthenticationService
logout(): Promise<void>;
session(): Promise<AuthenticationSession | undefined>;
disposeClient(client: AuthenticationServiceClient): void;
setOptions(authOptions: AuthOptions): void;
setOptions(authOptions: AuthOptions): Promise<void>;
initAuthSession(): Promise<void>;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export class ArduinoAuthenticationProvider implements AuthenticationProvider {
setInterval(checkToken, REFRESH_INTERVAL);
}

public setOptions(authOptions: AuthOptions) {
public async setOptions(authOptions: AuthOptions): Promise<void> {
this.authOptions = authOptions;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export class AuthenticationServiceImpl
protected readonly clients: AuthenticationServiceClient[] = [];
protected readonly toDispose = new DisposableCollection();

private initialized = false;
private initialized = false;

async onStart(): Promise<void> {
this.toDispose.pushAll([
Expand Down Expand Up @@ -49,12 +49,12 @@ export class AuthenticationServiceImpl
async initAuthSession(): Promise<void> {
if (!this.initialized) {
await this.delegate.init();
this.initialized = true
this.initialized = true;
}
}

setOptions(authOptions: AuthOptions) {
this.delegate.setOptions(authOptions);
setOptions(authOptions: AuthOptions): Promise<void> {
return this.delegate.setOptions(authOptions);
}

async login(): Promise<AuthenticationSession> {
Expand Down
9 changes: 9 additions & 0 deletions arduino-ide-extension/src/node/auth/keychain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,15 @@ export class Keychain {
return false;
}
try {
const stringifiedTokenLength = stringifiedToken.length;
const tokenLengthNotSupported =
stringifiedTokenLength > 2500 && process.platform === 'win32';

if (tokenLengthNotSupported) {
// TODO manage this specific error appropriately
return false;
}

await keytar.setPassword(
this.credentialsSection,
this.account,
Expand Down
10 changes: 9 additions & 1 deletion arduino-ide-extension/src/node/auth/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,15 @@ export function token2IToken(token: Token): IToken {
(token.id_token && jwt_decode(token.id_token)) || {};

return {
idToken: token.id_token,
/*
* ".id_token" is already decoded for account details above
* so we probably don't need to keep it around as "idToken".
* If we do, and subsequently try to store it with
* Windows Credential Manager (WCM) it's probable we'll
* exceed WCMs' 2500 password character limit breaking
* our auth functionality
*/
// ! idToken: token.id_token,
expiresIn: token.expires_in,
expiresAt: token.expires_in
? Date.now() + token.expires_in * 1000
Expand Down