Skip to content

Commit 333ad31

Browse files
committed
refactor(passport): align use of callbackURL with other strategies and user expectations
1 parent dba27f3 commit 333ad31

File tree

3 files changed

+59
-15
lines changed

3 files changed

+59
-15
lines changed

examples/passport.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
Strategy,
44
type VerifyFunction,
55
type StrategyOptions,
6+
type AuthenticateOptions,
67
} from 'openid-client/passport'
78

89
import express from 'express'
@@ -15,6 +16,11 @@ import { ensureLoggedIn, ensureLoggedOut } from 'connect-ensure-login'
1516

1617
let app!: express.Application
1718
let server!: URL // Authorization server's Issuer Identifier URL
19+
/**
20+
* In this example it is expected your application's origin + '/login' is
21+
* registered as an allowed redirect URL at the Authorization server
22+
*/
23+
let callbackURL!: URL
1824
let clientId!: string // Client identifier at the Authorization Server
1925
let clientSecret!: string // Client Secret
2026
let scope = 'openid email'
@@ -50,9 +56,10 @@ let verify: VerifyFunction = (tokens, verified) => {
5056
let options: StrategyOptions = {
5157
config,
5258
scope,
59+
callbackURL,
5360
}
5461

55-
passport.use(new Strategy(options, verify))
62+
passport.use('openid', new Strategy(options, verify))
5663

5764
passport.serializeUser((user: Express.User, cb) => {
5865
cb(null, user)
@@ -69,7 +76,9 @@ app.get('/', ensureLoggedIn('/login'), (req, res) => {
6976
app.get(
7077
'/login',
7178
ensureLoggedOut('/logout'),
72-
passport.authenticate(server.host, { successRedirect: '/' }),
79+
passport.authenticate('openid', {
80+
successRedirect: '/',
81+
} as AuthenticateOptions),
7382
)
7483

7584
app.get('/logout', (req, res) => {

src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3285,6 +3285,7 @@ export async function authorizationCodeGrant(
32853285
}
32863286
}
32873287
// TODO: what if searchParams *are* part of the registered redirect_uri?
3288+
// TODO: what if the registered redirect_uri has no pathname and no trailing /
32883289
redirectUri = stripParams(currentUrl)
32893290
switch (true) {
32903291
case !!jarm:

src/passport.ts

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,20 @@ export interface AuthenticateOptions extends passport.AuthenticateOptions {
7070
* @deprecated
7171
*/
7272
state?: never
73+
74+
/**
75+
* OAuth 2.0 redirect_uri to use for the request either for the authorization
76+
* request or token endpoint request, depending on whether it's part of
77+
* {@link Strategy.authenticate} options during the initial redirect or
78+
* callback phase.
79+
*
80+
* This is a request-specific override for {@link StrategyOptions.callbackURL}.
81+
*
82+
* Note: The option is called "callbackURL" to keep some continuity and
83+
* familiarity with other oauth-based strategies in the passport ecosystem,
84+
* namely "passport-oauth2".
85+
*/
86+
callbackURL?: URL | string
7387
}
7488

7589
/**
@@ -101,11 +115,13 @@ interface StrategyOptionsBase {
101115
*/
102116
DPoP?: getDPoPHandle
103117
/**
104-
* URL to which the authorization server will redirect the user after
105-
* obtaining authorization. This will be used as the `redirect_uri`
106-
* authorization request parameter unless specified elsewhere.
118+
* An absolute URL to which the authorization server will redirect the user
119+
* after obtaining authorization. The {@link !URL} instance's `href` will be
120+
* used as the `redirect_uri` authorization request and token endpoint request
121+
* parameters. When string is provided it will be internally casted to a
122+
* {@link URL} instance.
107123
*/
108-
callbackURL?: string
124+
callbackURL?: URL | string
109125
/**
110126
* OAuth 2.0 Authorization Request Scope. This will be used as the `scope`
111127
* authorization request parameter unless specified through other means.
@@ -192,7 +208,7 @@ export class Strategy implements passport.Strategy {
192208
/**
193209
* @internal
194210
*/
195-
_callbackURL?: string
211+
_callbackURL: Exclude<StrategyOptionsBase['callbackURL'], string>
196212
/**
197213
* @internal
198214
*/
@@ -253,7 +269,9 @@ export class Strategy implements passport.Strategy {
253269
this._useJAR = options.useJAR
254270
this._usePAR = options.usePAR
255271
this._verify = verify
256-
this._callbackURL = options.callbackURL
272+
if (options.callbackURL) {
273+
this._callbackURL = new URL(options.callbackURL)
274+
}
257275
this._passReqToCallback = options.passReqToCallback
258276
this._resource = options.resource
259277
this._authorizationDetails = options.authorizationDetails
@@ -297,6 +315,10 @@ export class Strategy implements passport.Strategy {
297315
setAuthorizationDetails(params, options.authorizationDetails)
298316
}
299317

318+
if (options?.callbackURL) {
319+
params.set('redirect_uri', new URL(options.callbackURL).href)
320+
}
321+
300322
return params
301323
}
302324

@@ -357,7 +379,7 @@ export class Strategy implements passport.Strategy {
357379
}
358380

359381
if (this._callbackURL && !redirectTo.searchParams.has('redirect_uri')) {
360-
redirectTo.searchParams.set('redirect_uri', this._callbackURL)
382+
redirectTo.searchParams.set('redirect_uri', this._callbackURL.href)
361383
}
362384

363385
if (this._scope && !redirectTo.searchParams.has('scope')) {
@@ -457,6 +479,14 @@ export class Strategy implements passport.Strategy {
457479
})
458480
}
459481

482+
if (options.callbackURL || this._callbackURL) {
483+
const _currentUrl = new URL(options.callbackURL! || this._callbackURL!)
484+
for (const [k, v] of currentUrl.searchParams.entries()) {
485+
_currentUrl.searchParams.append(k, v)
486+
}
487+
currentUrl = _currentUrl
488+
}
489+
460490
let input: URL | Request = currentUrl
461491
if (req.method === 'POST') {
462492
input = new Request(currentUrl.href, {
@@ -518,15 +548,19 @@ export class Strategy implements passport.Strategy {
518548
}
519549

520550
/**
521-
* Return the current request URL.
551+
* [Strategy method] Return the current request URL.
552+
*
553+
* This method is intended to be overloaded if its return value does not match
554+
* the actual URL the authorization server redirected the user to.
522555
*
523556
* - Its `searchParams` are used as the authorization response parameters when
524-
* the response type used by the client is `code`
525-
* - Its value stripped of `searchParams` and `hash` is used as the
526-
* `redirect_uri` authorization code grant token endpoint parameter
557+
* the authorization response request is a GET.
558+
* - Its resulting `href` value (after stripping its `searchParams` and `hash`)
559+
* is used as the `redirect_uri` authorization code grant token endpoint
560+
* parameter unless {@link AuthenticateOptions.callbackURL}, or
561+
* {@link StrategyOptionsBase.callbackURL} are used in which case those are
562+
* used as the `redirect_uri` parameter instead.
527563
*
528-
* This function may need to be overridden by users if its return value does
529-
* not match the actual URL the authorization server redirected the user to.
530564
*/
531565
currentUrl(req: express.Request): URL {
532566
return new URL(`${req.protocol}://${req.host}${req.originalUrl ?? req.url}`)

0 commit comments

Comments
 (0)