Skip to content

Commit fc0014c

Browse files
laurent22laurent22
committed
All: Open the connection screen when a SAML session has expired
1 parent 42d8df3 commit fc0014c

File tree

3 files changed

+75
-43
lines changed

3 files changed

+75
-43
lines changed

‎packages/lib/SyncTargetJoplinServer.ts‎

Lines changed: 46 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -78,58 +78,65 @@ export default class SyncTargetJoplinServer extends BaseSyncTarget {
7878
return super.fileApi();
7979
}
8080

81-
public static async checkConfig(options: FileApiOptions, syncTargetId: number = null) {
81+
public static async checkConfig(options: FileApiOptions, syncTargetId: number = null, fileApi: FileApi|null = null) {
8282
const output = {
8383
ok: false,
8484
errorMessage: '',
8585
};
8686

8787
syncTargetId = syncTargetId === null ? this.id() : syncTargetId;
8888

89-
let fileApi = null;
90-
try {
91-
fileApi = await newFileApi(syncTargetId, options);
92-
fileApi.requestRepeatCount_ = 0;
93-
} catch (error) {
94-
// If there's an error it's probably an application error, but we
95-
// can't proceed anyway, so exit.
96-
output.errorMessage = error.message;
97-
if (error.code) output.errorMessage += ` (Code ${error.code})`;
98-
return output;
89+
if (!fileApi) {
90+
try {
91+
fileApi = await newFileApi(syncTargetId, options);
92+
} catch (error) {
93+
// If there's an error it's probably an application error, but we
94+
// can't proceed anyway, so exit.
95+
output.errorMessage = error.message;
96+
if (error.code) output.errorMessage += ` (Code ${error.code})`;
97+
return output;
98+
}
9999
}
100100

101-
// First we try to fetch info.json. It may not be present if it's a new
102-
// sync target but otherwise, if it is, and it's valid, we know the
103-
// credentials are valid. We do this test first because it will work
104-
// even if account upload is disabled. And we need such account to
105-
// successfully login so that they can fix it by deleting extraneous
106-
// notes or resources.
101+
const previousRequestRepeatCount = fileApi.requestRepeatCount_;
102+
fileApi.requestRepeatCount_ = 0;
103+
107104
try {
108-
const r = await fileApi.get('info.json');
109-
if (r) {
110-
const parsed = JSON.parse(r);
111-
if (parsed) {
112-
output.ok = true;
113-
return output;
105+
// First we try to fetch info.json. It may not be present if it's a new
106+
// sync target but otherwise, if it is, and it's valid, we know the
107+
// credentials are valid. We do this test first because it will work
108+
// even if account upload is disabled. And we need such account to
109+
// successfully login so that they can fix it by deleting extraneous
110+
// notes or resources.
111+
try {
112+
const r = await fileApi.get('info.json');
113+
if (r) {
114+
const parsed = JSON.parse(r);
115+
if (parsed) {
116+
output.ok = true;
117+
return output;
118+
}
114119
}
120+
} catch (error) {
121+
// Ignore because we'll use the next test to check for sure if it
122+
// works or not.
123+
staticLogger.warn('Could not fetch or parse info.json:', error);
115124
}
116-
} catch (error) {
117-
// Ignore because we'll use the next test to check for sure if it
118-
// works or not.
119-
staticLogger.warn('Could not fetch or parse info.json:', error);
120-
}
121125

122-
// This is a more generic test, which writes a file and tries to read it
123-
// back.
124-
try {
125-
await fileApi.put('testing.txt', 'testing');
126-
const result = await fileApi.get('testing.txt');
127-
if (result !== 'testing') throw new Error(`Could not access data on server "${options.path()}"`);
128-
await fileApi.delete('testing.txt');
129-
output.ok = true;
130-
} catch (error) {
131-
output.errorMessage = error.message;
132-
if (error.code) output.errorMessage += ` (Code ${error.code})`;
126+
// This is a more generic test, which writes a file and tries to read it
127+
// back.
128+
try {
129+
await fileApi.put('testing.txt', 'testing');
130+
const result = await fileApi.get('testing.txt');
131+
if (result !== 'testing') throw new Error(`Could not access data on server "${options.path()}"`);
132+
await fileApi.delete('testing.txt');
133+
output.ok = true;
134+
} catch (error) {
135+
output.errorMessage = error.message;
136+
if (error.code) output.errorMessage += ` (Code ${error.code})`;
137+
}
138+
} finally {
139+
fileApi.requestRepeatCount_ = previousRequestRepeatCount;
133140
}
134141

135142
return output;

‎packages/lib/SyncTargetJoplinServerSAML.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ export const authenticateWithCode = async (code: string) => {
5252
//
5353
// Based on the regular Joplin Server sync target.
5454
export default class SyncTargetJoplinServerSAML extends SyncTargetJoplinServer {
55+
56+
private lastFileApiOptions_: FileApiOptions|null = null;
57+
5558
public static override id() {
5659
return 11;
5760
}
@@ -65,7 +68,16 @@ export default class SyncTargetJoplinServerSAML extends SyncTargetJoplinServer {
6568
}
6669

6770
public override async isAuthenticated() {
68-
return Setting.value('sync.11.id') !== '';
71+
if (!Setting.value('sync.11.id')) return false;
72+
73+
// We check that the file API has been initialized at least once, otherwise the below check
74+
// will always fail and it will be impossible to login.
75+
if (this.lastFileApiOptions_) {
76+
const check = await SyncTargetJoplinServer.checkConfig(null, null, await this.fileApi());
77+
return check.ok;
78+
}
79+
80+
return true;
6981
}
7082

7183
public static override requiresPassword() {
@@ -111,11 +123,12 @@ export default class SyncTargetJoplinServerSAML extends SyncTargetJoplinServer {
111123
}
112124

113125
protected override async initFileApi() {
114-
return initFileApi(SyncTargetJoplinServerSAML.id(), this.logger(), {
126+
this.lastFileApiOptions_ = {
115127
path: () => Setting.value('sync.11.path'),
116128
userContentPath: () => Setting.value('sync.11.userContentPath'),
117129
username: () => '',
118130
password: () => '',
119-
});
131+
};
132+
return initFileApi(SyncTargetJoplinServerSAML.id(), this.logger(), this.lastFileApiOptions_);
120133
}
121134
}

‎packages/lib/commands/synchronize.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { utils, CommandRuntime, CommandDeclaration, CommandContext } from '../se
22
import { _ } from '../locale';
33
import { reg } from '../registry';
44
import Setting from '../models/Setting';
5+
import NavService from '../services/NavService';
56

67
export const declaration: CommandDeclaration = {
78
name: 'synchronize',
@@ -35,7 +36,18 @@ export const runtime = (): CommandRuntime => {
3536
return 'auth';
3637
}
3738

38-
reg.logger().error('Not authenticated with sync target - please check your credentials.');
39+
const error = new Error('Not authenticated with sync target - please check your credentials.');
40+
41+
utils.store.dispatch({
42+
type: 'SYNC_REPORT_UPDATE',
43+
report: { errors: [error] },
44+
});
45+
46+
if (Setting.value('sync.target') === 11) {
47+
await NavService.go('JoplinServerSamlLogin');
48+
}
49+
50+
reg.logger().error(error);
3951
return 'error';
4052
}
4153

0 commit comments

Comments
 (0)