Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit e4715e4

Browse files
committed
prevent empty code challenge and undo refactor
1 parent 0bf1061 commit e4715e4

File tree

8 files changed

+21
-164
lines changed

8 files changed

+21
-164
lines changed

‎lib/grant-types/abstract-grant-type.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ function AbstractGrantType(options) {
3030
this.model = options.model;
3131
this.refreshTokenLifetime = options.refreshTokenLifetime;
3232
this.alwaysIssueNewRefreshToken = options.alwaysIssueNewRefreshToken;
33-
this.PKCEEnabled = options.PKCEEnabled;
3433
}
3534

3635
/**

‎lib/grant-types/authorization-code-grant-type.js‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -122,20 +122,15 @@ AuthorizationCodeGrantType.prototype.getAuthorizationCode = function(request, cl
122122
throw new InvalidGrantError('Invalid grant: `redirect_uri` is not a valid URI');
123123
}
124124

125-
if (this.PKCEEnabled && client.isPublic) {
126-
if (!code.codeChallenge) {
127-
throw new InvalidGrantError('Invalid grant: code challenge is invalid');
128-
}
129-
130-
/**
131-
* The "code_challenge_method" is bound to the Authorization Code when
132-
* the Authorization Code is issued. That is the method that the token
133-
* endpoint MUST use to verify the "code_verifier".
134-
*/
125+
if (code.codeChallenge) {
135126
if (!code.codeChallengeMethod) {
136127
throw new ServerError('Server error: `getAuthorizationCode()` did not return a `codeChallengeMethod` property');
137128
}
138129

130+
if (!request.body.code_verifier) {
131+
throw new InvalidGrantError('Missing parameter: `code_verifier`');
132+
}
133+
139134
if (code.codeChallengeMethod === 'plain' && code.codeChallenge !== request.body.code_verifier) {
140135
throw new InvalidGrantError('Invalid grant: code verifier is invalid');
141136
}

‎lib/handlers/authorize-handler.js‎

Lines changed: 9 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ function AuthorizeHandler(options) {
6262
this.allowEmptyState = options.allowEmptyState;
6363
this.authenticateHandler = options.authenticateHandler || new AuthenticateHandler(options);
6464
this.authorizationCodeLifetime = options.authorizationCodeLifetime;
65-
this.PKCEEnabled = options.PKCEEnabled;
6665
this.model = options.model;
6766
}
6867

@@ -99,17 +98,11 @@ AuthorizeHandler.prototype.handle = function(request, response) {
9998
var scope;
10099
var state;
101100
var ResponseType;
102-
var codeChallenge;
103-
var codeChallengeMethod;
101+
var codeChallenge=this.getCodeChallenge(request);
102+
var codeChallengeMethod=codeChallenge&&this.getCodeChallengeMethod(request);
104103

105104
return Promise.bind(this)
106105
.then(function() {
107-
codeChallenge = this.getCodeChallenge(request, client);
108-
109-
if (codeChallenge) {
110-
codeChallengeMethod = this.getCodeChallengeMethod(request);
111-
}
112-
113106
var requestedScope = this.getScope(request);
114107

115108
return this.validateScope(user, client, requestedScope);
@@ -157,19 +150,15 @@ AuthorizeHandler.prototype.generateAuthorizationCode = function(client, user, sc
157150
return tokenUtil.generateRandomToken();
158151
};
159152

160-
AuthorizeHandler.prototype.getCodeChallenge = function(request,client) {
153+
AuthorizeHandler.prototype.getCodeChallenge = function(request) {
161154
var codeChallenge = request.body.code_challenge || request.query.code_challenge;
162155

163-
/**
164-
* If the server requires Proof Key for Code Exchange (PKCE) by OAuth
165-
* public clients and the client does not send the "code_challenge" in
166-
* the request, the authorization endpoint MUST return the authorization
167-
* error response with the "error" value set to "invalid_request". The
168-
* "error_description" or the response of "error_uri" SHOULD explain the
169-
* nature of error, e.g., code challenge required.
170-
*/
171-
if (this.PKCEEnabled && client.isPublic && _.isEmpty(codeChallenge)) {
172-
throw new InvalidRequestError('Missing parameter: `code_challenge`. Public clients must include a code_challenge');
156+
if (!codeChallenge || codeChallenge === '') {
157+
return;
158+
}
159+
160+
if (!is.vschar(codeChallenge)) {
161+
throw new InvalidRequestError('Invalid parameter: `code_challenge`');
173162
}
174163

175164
return codeChallenge;
@@ -217,7 +206,6 @@ AuthorizeHandler.prototype.getClient = function(request) {
217206
throw new InvalidRequestError('Invalid request: `redirect_uri` is not a valid URI');
218207
}
219208
return promisify(this.model.getClient, 2).call(this.model, clientId, null)
220-
.bind(this)
221209
.then(function(client) {
222210
if (!client) {
223211
throw new InvalidClientError('Invalid client: client credentials are invalid');
@@ -239,15 +227,6 @@ AuthorizeHandler.prototype.getClient = function(request) {
239227
throw new InvalidClientError('Invalid client: `redirect_uri` does not match client value');
240228
}
241229

242-
if (this.PKCEEnabled) {
243-
if (client.isPublic === undefined) {
244-
throw new InvalidClientError('Invalid client: missing client `isPublic`');
245-
}
246-
247-
if (typeof client.isPublic !== 'boolean') {
248-
throw new InvalidClientError('Invalid client: `isPublic` must be a boolean');
249-
}
250-
}
251230
return client;
252231
});
253232
};

‎lib/handlers/token-handler.js‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ function TokenHandler(options) {
6262
this.allowExtendedTokenAttributes = options.allowExtendedTokenAttributes;
6363
this.requireClientAuthentication = options.requireClientAuthentication || {};
6464
this.alwaysIssueNewRefreshToken = options.alwaysIssueNewRefreshToken !== false;
65-
this.PKCEEnabled = options.PKCEEnabled;
6665
}
6766

6867
/**
@@ -137,7 +136,6 @@ TokenHandler.prototype.getClient = function(request, response) {
137136
}
138137

139138
return promisify(this.model.getClient, 2).call(this.model, credentials.clientId, credentials.clientSecret)
140-
.bind(this)
141139
.then(function(client) {
142140
if (!client) {
143141
throw new InvalidClientError('Invalid client: client is invalid');
@@ -151,16 +149,6 @@ TokenHandler.prototype.getClient = function(request, response) {
151149
throw new ServerError('Server error: `grants` must be an array');
152150
}
153151

154-
if (this.PKCEEnabled) {
155-
if (client.isPublic === undefined) {
156-
throw new ServerError('Server error: missing client `isPublic`');
157-
}
158-
159-
if (typeof client.isPublic !== 'boolean') {
160-
throw new ServerError('Server error: invalid client, `isPublic` must be a boolean');
161-
}
162-
}
163-
164152
return client;
165153
})
166154
.catch(function(e) {
@@ -239,8 +227,7 @@ TokenHandler.prototype.handleGrantType = function(request, client) {
239227
accessTokenLifetime: accessTokenLifetime,
240228
model: this.model,
241229
refreshTokenLifetime: refreshTokenLifetime,
242-
alwaysIssueNewRefreshToken: this.alwaysIssueNewRefreshToken,
243-
PKCEenabled: this.PKCEenabled
230+
alwaysIssueNewRefreshToken: this.alwaysIssueNewRefreshToken
244231
};
245232

246233
return new Type(options)

‎lib/server.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,7 @@ OAuth2Server.prototype.authenticate = function(request, response, options, callb
5151
OAuth2Server.prototype.authorize = function(request, response, options, callback) {
5252
options = _.assign({
5353
allowEmptyState: false,
54-
authorizationCodeLifetime: 5 * 60, // 5 minutes.
55-
PKCEEnabled: false
54+
authorizationCodeLifetime: 5 * 60 // 5 minutes.
5655
}, this.options, options);
5756

5857
return new AuthorizeHandler(options)
@@ -69,8 +68,7 @@ OAuth2Server.prototype.token = function(request, response, options, callback) {
6968
accessTokenLifetime: 60 * 60, // 1 hour.
7069
refreshTokenLifetime: 60 * 60 * 24 * 14, // 2 weeks.
7170
allowExtendedTokenAttributes: false,
72-
requireClientAuthentication: {}, // defaults to true for all grant types
73-
PKCEEnabled: false
71+
requireClientAuthentication: {} // defaults to true for all grant types
7472
}, this.options, options);
7573

7674
return new TokenHandler(options)

‎test/integration/grant-types/authorization-code-grant-type_test.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ describe('AuthorizationCodeGrantType integration', function() {
379379
revokeAuthorizationCode: function() {},
380380
saveToken: function() {}
381381
};
382-
var grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model,PKCEEnabled: true });
382+
var grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model });
383383
var request = new Request({ body: { code: 12345, code_verifier: 'foo' }, headers: {}, method: {}, query: {} });
384384

385385
return grantType.getAuthorizationCode(request, client)
@@ -405,7 +405,7 @@ describe('AuthorizationCodeGrantType integration', function() {
405405
revokeAuthorizationCode: function() {},
406406
saveToken: function() {}
407407
};
408-
var grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model,PKCEEnabled: true });
408+
var grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model });
409409
var request = new Request({ body: { code: 12345, code_verifier: 'foo' }, headers: {}, method: {}, query: {} });
410410

411411
return grantType.getAuthorizationCode(request, client)
@@ -432,7 +432,7 @@ describe('AuthorizationCodeGrantType integration', function() {
432432
revokeAuthorizationCode: function() {},
433433
saveToken: function() {}
434434
};
435-
var grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model,PKCEEnabled: true });
435+
var grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model });
436436
var request = new Request({ body: { code: 12345, code_verifier: codeVerifier }, headers: {}, method: {}, query: {} });
437437

438438
return grantType.getAuthorizationCode(request, client)
@@ -457,7 +457,7 @@ describe('AuthorizationCodeGrantType integration', function() {
457457
revokeAuthorizationCode: function() {},
458458
saveToken: function() {}
459459
};
460-
var grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model,PKCEEnabled: true });
460+
var grantType = new AuthorizationCodeGrantType({ accessTokenLifetime: 123, model: model });
461461
var request = new Request({ body: { code: 12345, code_verifier: 'baz' }, headers: {}, method: {}, query: {} });
462462

463463
return grantType.getAuthorizationCode(request, client)

‎test/integration/handlers/authorize-handler_test.js‎

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -783,44 +783,6 @@ describe('AuthorizeHandler integration', function() {
783783
e.message.should.equal('Invalid client: `redirect_uri` does not match client value');
784784
});
785785
});
786-
describe('with PKCEEnabled', function() {
787-
it('should throw an error if `client.isPublic` is missing`', function() {
788-
var model = {
789-
getAccessToken: function() {},
790-
getClient: function() {
791-
return { grants: ['authorization_code'], redirectUris: ['https://example.com'] };
792-
},
793-
saveAuthorizationCode: function() {}
794-
};
795-
var handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model, PKCEEnabled: true });
796-
var request = new Request({ body: { client_id: 12345, response_type: 'code', redirect_uri: 'https://example.com' }, headers: {}, method: {}, query: {} });
797-
798-
return handler.getClient(request)
799-
.then(should.fail)
800-
.catch(function(e) {
801-
e.should.be.an.instanceOf(InvalidClientError);
802-
e.message.should.equal('Invalid client: missing client `isPublic`');
803-
});
804-
});
805-
it('should throw an error if `client.isPublic` is invalid`', function() {
806-
var model = {
807-
getAccessToken: function() {},
808-
getClient: function() {
809-
return { grants: ['authorization_code'], redirectUris: ['https://example.com'], isPublic: 'foo' };
810-
},
811-
saveAuthorizationCode: function() {}
812-
};
813-
var handler = new AuthorizeHandler({ authorizationCodeLifetime: 120, model: model, PKCEEnabled: true });
814-
var request = new Request({ body: { client_id: 12345, response_type: 'code', redirect_uri: 'https://example.com' }, headers: {}, method: {}, query: {} });
815-
816-
return handler.getClient(request)
817-
.then(should.fail)
818-
.catch(function(e) {
819-
e.should.be.an.instanceOf(InvalidClientError);
820-
e.message.should.equal('Invalid client: `isPublic` must be a boolean');
821-
});
822-
});
823-
});
824786

825787
it('should support promises', function() {
826788
var model = {

‎test/integration/handlers/token-handler_test.js‎

Lines changed: 0 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -114,16 +114,6 @@ describe('TokenHandler integration', function() {
114114
handler.alwaysIssueNewRefreshToken.should.equal(true);
115115
});
116116

117-
it('should set the `PKCEEnabled` to true', function() {
118-
var model = {
119-
getClient: function() {},
120-
saveToken: function() {}
121-
};
122-
var handler = new TokenHandler({ accessTokenLifetime: 123, model: model, refreshTokenLifetime: 120, PKCEEnabled: true });
123-
124-
handler.PKCEEnabled.should.equal(true);
125-
});
126-
127117
it('should set the `extendedGrantTypes`', function() {
128118
var extendedGrantTypes = { foo: 'bar' };
129119
var model = {
@@ -595,59 +585,6 @@ describe('TokenHandler integration', function() {
595585
.catch(should.fail);
596586
});
597587
});
598-
describe('with PKCE enabled', function() {
599-
it('should return a client with `isPublic` parameter', function() {
600-
var client = { id: 12345, grants: [], isPublic: true };
601-
var model = {
602-
getClient: function() { return client; },
603-
saveToken: function() {}
604-
};
605-
606-
var handler = new TokenHandler({
607-
accessTokenLifetime: 120,
608-
model: model,
609-
refreshTokenLifetime: 120,
610-
PKCEEnabled: true
611-
});
612-
var request = new Request({ body: { client_id: 'foo', client_secret: 'baz', grant_type: 'authorization_code' }, headers: {}, method: {}, query: {} });
613-
614-
return handler.getClient(request)
615-
.then(function(data) {
616-
data.should.equal(client);
617-
})
618-
.catch(should.fail);
619-
});
620-
it('should throw an error if `client.isPublic` is invalid', function() {
621-
var model = {
622-
getClient: function() { return { id: 123, grants: [], isPublic: 'foo' }; },
623-
saveToken: function() {}
624-
};
625-
var handler = new TokenHandler({ accessTokenLifetime: 120, model: model, refreshTokenLifetime: 120, PKCEEnabled: true });
626-
var request = new Request({ body: { client_id: 12345, client_secret: 'secret' }, headers: {}, method: {}, query: {} });
627-
628-
return handler.getClient(request)
629-
.then(should.fail)
630-
.catch(function(e) {
631-
e.should.be.an.instanceOf(ServerError);
632-
e.message.should.equal('Server error: invalid client, `isPublic` must be a boolean');
633-
});
634-
});
635-
it('should throw an error if `client.isPublic` is missing', function() {
636-
var model = {
637-
getClient: function() { return { id: 123, grants: [] }; },
638-
saveToken: function() {}
639-
};
640-
var handler = new TokenHandler({ accessTokenLifetime: 120, model: model, refreshTokenLifetime: 120, PKCEEnabled: true });
641-
var request = new Request({ body: { client_id: 12345, client_secret: 'secret' }, headers: {}, method: {}, query: {} });
642-
643-
return handler.getClient(request)
644-
.then(should.fail)
645-
.catch(function(e) {
646-
e.should.be.an.instanceOf(ServerError);
647-
e.message.should.equal('Server error: missing client `isPublic`');
648-
});
649-
});
650-
});
651588

652589
it('should support promises', function() {
653590
var model = {

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /