Skip to content

Commit 96e035f

Browse files
committed
fix: registered native loopback redirect_uris do not get normalized
1 parent d7b927d commit 96e035f

File tree

4 files changed

+28
-34
lines changed

4 files changed

+28
-34
lines changed

lib/consts/client_attributes.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ const HTTPS_URI = [
147147
'sector_identifier_uri',
148148
];
149149

150-
const LOOPBACKS = ['localhost', '127.0.0.1', '[::1]'];
150+
const LOOPBACKS = new Set(['localhost', '127.0.0.1', '[::1]']);
151151

152152
const ENUM = {
153153
application_type: () => ['native', 'web'],

lib/helpers/client_schema.js

+2-18
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,6 @@ module.exports = function getSchema(provider) {
248248
this.postLogoutRedirectUris();
249249
this.redirectUris();
250250
this.webMessageUris();
251-
this.normalizeNativeAppUris();
252251
this.checkContacts();
253252

254253
// max_age and client_secret_expires_at format
@@ -473,21 +472,6 @@ module.exports = function getSchema(provider) {
473472
this.response_types = this.response_types.map((type) => [...new Set(type.split(' '))].sort().join(' '));
474473
}
475474

476-
normalizeNativeAppUris() {
477-
if (this.application_type === 'web') return;
478-
479-
this.redirect_uris = this.redirect_uris.map((redirectUri) => {
480-
const parsed = new url.URL(redirectUri);
481-
// remove the port component, making dynamic ports allowed for loopback uris
482-
if (parsed.protocol === 'http:' && LOOPBACKS.includes(parsed.hostname)) {
483-
parsed.port = 80; // http + 80 = no port part in the string
484-
return parsed.href;
485-
}
486-
487-
return redirectUri;
488-
});
489-
}
490-
491475
postLogoutRedirectUris() {
492476
this.post_logout_redirect_uris.forEach((uri) => {
493477
try {
@@ -554,12 +538,12 @@ module.exports = function getSchema(provider) {
554538
case 'native': {
555539
switch (protocol) {
556540
case 'http:': // Loopback Interface Redirection
557-
if (!LOOPBACKS.includes(hostname)) {
541+
if (!LOOPBACKS.has(hostname)) {
558542
this.invalidate('redirect_uris for native clients using http as a protocol can only use loopback addresses as hostnames');
559543
}
560544
break;
561545
case 'https:': // Claimed HTTPS URI Redirection
562-
if (LOOPBACKS.includes(hostname)) {
546+
if (LOOPBACKS.has(hostname)) {
563547
this.invalidate(`redirect_uris for native clients using claimed HTTPS URIs must not be using ${hostname} as hostname`);
564548
}
565549
break;

lib/models/client.js

+22-12
Original file line numberDiff line numberDiff line change
@@ -436,22 +436,32 @@ module.exports = function getClient(provider) {
436436
return this.grantTypes.includes(type);
437437
}
438438

439-
redirectUriAllowed(redirectUri) {
440-
let checkedUri = redirectUri;
439+
redirectUriAllowed(value) {
440+
let parsed;
441+
try {
442+
parsed = new URL(value);
443+
} catch (err) {
444+
return false;
445+
}
446+
447+
const match = this.redirectUris.includes(value);
441448
if (
442-
this.applicationType === 'native'
443-
&& redirectUri.startsWith('http:')
449+
match
450+
|| this.applicationType !== 'native'
451+
|| !parsed.protocol === 'http:'
452+
|| !LOOPBACKS.has(parsed.hostname)
444453
) {
445-
try {
446-
const parsed = new URL(redirectUri);
447-
if (LOOPBACKS.includes(parsed.hostname)) {
448-
parsed.port = 80;
449-
checkedUri = parsed.href;
450-
}
451-
} catch (err) {}
454+
return match;
452455
}
453456

454-
return this.redirectUris.includes(checkedUri);
457+
parsed.port = '';
458+
459+
return !!this.redirectUris
460+
.find((registeredUri) => {
461+
const registered = new URL(registeredUri);
462+
registered.port = '';
463+
return parsed.href === registered.href;
464+
});
455465
}
456466

457467
checkSessionOriginAllowed(origin) {

test/oauth_native_apps/oauth_native_apps.test.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ describe('OAuth 2.0 for Native Apps Best Current Practice features', () => {
8787
token_endpoint_auth_method: 'none',
8888
redirect_uris: ['http://localhost:2355/op/callback'],
8989
}).then((client) => {
90-
expect(client.redirectUris).to.contain('http://localhost/op/callback');
90+
expect(client.redirectUris).to.contain('http://localhost:2355/op/callback');
9191
expect(client.redirectUriAllowed('http://localhost/op/callback')).to.be.true;
9292
expect(client.redirectUriAllowed('http://localhost:80/op/callback')).to.be.true;
9393
expect(client.redirectUriAllowed('http://localhost:443/op/callback')).to.be.true;
@@ -123,7 +123,7 @@ describe('OAuth 2.0 for Native Apps Best Current Practice features', () => {
123123
token_endpoint_auth_method: 'none',
124124
redirect_uris: ['http://127.0.0.1:2355/op/callback'],
125125
}).then((client) => {
126-
expect(client.redirectUris).to.contain('http://127.0.0.1/op/callback');
126+
expect(client.redirectUris).to.contain('http://127.0.0.1:2355/op/callback');
127127
expect(client.redirectUriAllowed('http://127.0.0.1/op/callback')).to.be.true;
128128
expect(client.redirectUriAllowed('http://127.0.0.1:80/op/callback')).to.be.true;
129129
expect(client.redirectUriAllowed('http://127.0.0.1:443/op/callback')).to.be.true;
@@ -159,7 +159,7 @@ describe('OAuth 2.0 for Native Apps Best Current Practice features', () => {
159159
token_endpoint_auth_method: 'none',
160160
redirect_uris: ['http://[::1]:2355/op/callback'],
161161
}).then((client) => {
162-
expect(client.redirectUris).to.contain('http://[::1]/op/callback');
162+
expect(client.redirectUris).to.contain('http://[::1]:2355/op/callback');
163163
expect(client.redirectUriAllowed('http://[::1]/op/callback')).to.be.true;
164164
expect(client.redirectUriAllowed('http://[::1]:80/op/callback')).to.be.true;
165165
expect(client.redirectUriAllowed('http://[::1]:443/op/callback')).to.be.true;

0 commit comments

Comments
 (0)