Skip to content

Commit

Permalink
client config: make state parameter optional
Browse files Browse the repository at this point in the history
The state parameter is recommended but not mandatory so make this
configurable in case the server does not support it. By default this is
set to true in order to prevent cross-site request forgery.

There is a pending PR in the main repo p2#284
  • Loading branch information
balland authored and ajandrade committed Dec 28, 2022
1 parent 30f46ad commit da64c49
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 7 deletions.
15 changes: 9 additions & 6 deletions Sources/Base/OAuth2Base.swift
Original file line number Diff line number Diff line change
Expand Up @@ -420,12 +420,15 @@ open class OAuth2Base: OAuth2Securable {
This method checks `state`, throws `OAuth2Error.missingState` or `OAuth2Error.invalidState`. Resets state if it matches.
*/
public final func assureMatchesState(_ params: OAuth2JSON) throws {
guard let state = params["state"] as? String, !state.isEmpty else {
throw OAuth2Error.missingState
}
logger?.trace("OAuth2", msg: "Checking state, got “\(state)”, expecting “\(context.state)")
if !context.matchesState(state) {
throw OAuth2Error.invalidState
if let state = params["state"] as? String, !state.isEmpty {
logger?.trace("OAuth2", msg: "Checking state, got “\(state)”, expecting “\(context.state)")
if !context.matchesState(state) {
throw OAuth2Error.invalidState
}
} else {
if !clientConfig.stateParameterOptional {
throw OAuth2Error.missingState
}
}
context.resetState()
}
Expand Down
7 changes: 6 additions & 1 deletion Sources/Base/OAuth2ClientConfig.swift
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ open class OAuth2ClientConfig {

/// Add custom parameters to the authorization request.
public var customParameters: [String: String]? = nil

/// Whether the state parameter is optional
public var stateParameterOptional = false

/// Most servers use UTF-8 encoding for Authorization headers, but that's not 100% true: make it configurable (see https://github.com/p2/OAuth2/issues/165).
open var authStringEncoding = String.Encoding.utf8
Expand Down Expand Up @@ -146,7 +149,9 @@ open class OAuth2ClientConfig {
if let params = settings["parameters"] as? OAuth2StringDict {
customParameters = params
}

if let stateOptional = settings["state_parameter_optional"] as? Bool {
stateParameterOptional = stateOptional
}
// access token options
if let assume = settings["token_assume_unexpired"] as? Bool {
accessTokenAssumeUnexpired = assume
Expand Down
22 changes: 22 additions & 0 deletions Tests/FlowTests/OAuth2CodeGrantTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,28 @@ class OAuth2CodeGrantTests: XCTestCase {
XCTAssertTrue(false, "Should not throw, but threw \(error)")
}
}

func testRedirectURINoStateParameterAllowed() {
let settings: OAuth2JSON = [
"client_id": "abc",
"client_secret": "xyz",
"authorize_uri": "https://auth.ful.io",
"token_uri": "https://token.ful.io",
"keychain": false,
"state_parameter_optional": true
]
let oauth = OAuth2CodeGrant(settings: settings)
oauth.redirect = "oauth2://callback"
oauth.context.redirectURL = oauth.redirect
// parse no state
let redirect = URL(string: "oauth2://callback?code=C0D3")!
do {
_ = try oauth.validateRedirectURL(redirect)
}
catch let error {
XCTAssertTrue(false, "Must not end up here with \(error)")
}
}

func testTokenRequest() {
let oauth = OAuth2CodeGrant(settings: [
Expand Down

0 comments on commit da64c49

Please sign in to comment.