Skip to content

Commit 9ddff90

Browse files
jaredhansonjaredhanson
committed
Support authorization endpoint that has query parameters, per RFC section 3.1.
1 parent 1eb4f22 commit 9ddff90

File tree

5 files changed

+147
-4
lines changed

5 files changed

+147
-4
lines changed

‎lib/strategy.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,14 +225,23 @@ OAuth2Strategy.prototype.authenticate = function(req, options) {
225225
var state = options.state;
226226
if (state) {
227227
params.state = state;
228-
var location = this._oauth2.getAuthorizeUrl(params);
228+
229+
var parsed = url.parse(this._oauth2._authorizeUrl, true);
230+
utils.merge(parsed.query, params);
231+
parsed.query['client_id'] = this._oauth2._clientId;
232+
delete parsed.search;
233+
var location = url.format(parsed);
229234
this.redirect(location);
230235
} else {
231236
function stored(err, state) {
232237
if (err) { return self.error(err); }
233238

234239
if (state) { params.state = state; }
235-
var location = self._oauth2.getAuthorizeUrl(params);
240+
var parsed = url.parse(self._oauth2._authorizeUrl, true);
241+
utils.merge(parsed.query, params);
242+
parsed.query['client_id'] = self._oauth2._clientId;
243+
delete parsed.search;
244+
var location = url.format(parsed);
236245
self.redirect(location);
237246
}
238247

‎lib/utils.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
exports.merge = require('utils-merge');
2+
13
/**
24
* Reconstructs the original URL of the request.
35
*

‎package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,10 @@
3333
],
3434
"main": "./lib",
3535
"dependencies": {
36-
"passport-strategy": "1.x.x",
3736
"oauth": "0.9.x",
38-
"uid2": "0.0.x"
37+
"passport-strategy": "1.x.x",
38+
"uid2": "0.0.x",
39+
"utils-merge": "1.x.x"
3940
},
4041
"devDependencies": {
4142
"make-node": "0.3.x",

‎test/oauth2.state.session.test.js

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,50 @@ describe('OAuth2Strategy', function() {
109109

110110
}); // issuing authorization request
111111

112+
describe('issuing authorization request to authorization server using authorization endpoint that has query parameters including state', function() {
113+
var strategy = new OAuth2Strategy({
114+
authorizationURL: 'https://www.example.com/oauth2/authorize?foo=bar&state=baz',
115+
tokenURL: 'https://www.example.com/oauth2/token',
116+
clientID: 'ABC123',
117+
clientSecret: 'secret',
118+
callbackURL: 'https://www.example.net/auth/example/callback',
119+
state: true
120+
},
121+
function(accessToken, refreshToken, profile, done) {});
122+
123+
124+
describe('that redirects to service provider', function() {
125+
var request, url;
126+
127+
before(function(done) {
128+
chai.passport.use(strategy)
129+
.redirect(function(u) {
130+
url = u;
131+
done();
132+
})
133+
.req(function(req) {
134+
request = req;
135+
req.session = {};
136+
})
137+
.authenticate();
138+
});
139+
140+
it('should be redirected', function() {
141+
var u = uri.parse(url, true);
142+
expect(u.query.foo).equal('bar');
143+
expect(u.query.state).to.have.length(24);
144+
});
145+
146+
it('should save state in session', function() {
147+
var u = uri.parse(url, true);
148+
149+
expect(request.session['oauth2:www.example.com'].state).to.have.length(24);
150+
expect(request.session['oauth2:www.example.com'].state).to.equal(u.query.state);
151+
});
152+
}); // that redirects to service provider
153+
154+
}); // issuing authorization request to authorization server using authorization endpoint that has query parameters including state
155+
112156
describe('processing response to authorization request', function() {
113157
var strategy = new OAuth2Strategy({
114158
authorizationURL: 'https://www.example.com/oauth2/authorize',

‎test/oauth2.test.js

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,93 @@ describe('OAuth2Strategy', function() {
361361
});
362362
}); // that redirects to service provider with relative redirect URI option
363363

364+
describe('that redirects to authorization server using authorization endpoint that has query parameters with scope option', function() {
365+
var strategy = new OAuth2Strategy({
366+
authorizationURL: 'https://www.example.com/oauth2/authorize?foo=bar',
367+
tokenURL: 'https://www.example.com/oauth2/token',
368+
clientID: 'ABC123',
369+
clientSecret: 'secret',
370+
callbackURL: 'https://www.example.net/auth/example/callback',
371+
},
372+
function(accessToken, refreshToken, profile, done) {});
373+
374+
375+
var url;
376+
377+
before(function(done) {
378+
chai.passport.use(strategy)
379+
.redirect(function(u) {
380+
url = u;
381+
done();
382+
})
383+
.req(function(req) {
384+
})
385+
.authenticate({ scope: 'email' });
386+
});
387+
388+
it('should be redirected', function() {
389+
expect(url).to.equal('https://www.example.com/oauth2/authorize?foo=bar&response_type=code&redirect_uri=https%3A%2F%2Fwww.example.net%2Fauth%2Fexample%2Fcallback&scope=email&client_id=ABC123');
390+
});
391+
}); // that redirects to authorization server using authorization endpoint that has query parameters with scope option
392+
393+
describe('that redirects to authorization server using authorization endpoint that has query parameters including scope with scope option', function() {
394+
var strategy = new OAuth2Strategy({
395+
authorizationURL: 'https://www.example.com/oauth2/authorize?foo=bar&scope=baz',
396+
tokenURL: 'https://www.example.com/oauth2/token',
397+
clientID: 'ABC123',
398+
clientSecret: 'secret',
399+
callbackURL: 'https://www.example.net/auth/example/callback',
400+
},
401+
function(accessToken, refreshToken, profile, done) {});
402+
403+
404+
var url;
405+
406+
before(function(done) {
407+
chai.passport.use(strategy)
408+
.redirect(function(u) {
409+
url = u;
410+
done();
411+
})
412+
.req(function(req) {
413+
})
414+
.authenticate({ scope: 'email' });
415+
});
416+
417+
it('should be redirected', function() {
418+
expect(url).to.equal('https://www.example.com/oauth2/authorize?foo=bar&scope=email&response_type=code&redirect_uri=https%3A%2F%2Fwww.example.net%2Fauth%2Fexample%2Fcallback&client_id=ABC123');
419+
});
420+
}); // that redirects to authorization server using authorization endpoint that has query parameters including scope with scope option
421+
422+
describe('that redirects to authorization server using authorization endpoint that has query parameters including state with state option', function() {
423+
var strategy = new OAuth2Strategy({
424+
authorizationURL: 'https://www.example.com/oauth2/authorize?foo=bar&state=baz',
425+
tokenURL: 'https://www.example.com/oauth2/token',
426+
clientID: 'ABC123',
427+
clientSecret: 'secret',
428+
callbackURL: 'https://www.example.net/auth/example/callback',
429+
},
430+
function(accessToken, refreshToken, profile, done) {});
431+
432+
433+
var url;
434+
435+
before(function(done) {
436+
chai.passport.use(strategy)
437+
.redirect(function(u) {
438+
url = u;
439+
done();
440+
})
441+
.req(function(req) {
442+
})
443+
.authenticate({ state: 'foo123' });
444+
});
445+
446+
it('should be redirected', function() {
447+
expect(url).to.equal('https://www.example.com/oauth2/authorize?foo=bar&state=foo123&response_type=code&redirect_uri=https%3A%2F%2Fwww.example.net%2Fauth%2Fexample%2Fcallback&client_id=ABC123');
448+
});
449+
}); // that redirects to authorization server using authorization endpoint that has query parameters including state with state option
450+
364451
}); // issuing authorization request
365452

366453

0 commit comments

Comments
 (0)