diff options
| -rw-r--r-- | Auth/OpenID/Consumer.php | 57 | ||||
| -rw-r--r-- | Tests/Auth/OpenID/VerifyDisco.php | 35 |
2 files changed, 74 insertions, 18 deletions
diff --git a/Auth/OpenID/Consumer.php b/Auth/OpenID/Consumer.php index 93bd929..52fb100 100644 --- a/Auth/OpenID/Consumer.php +++ b/Auth/OpenID/Consumer.php @@ -986,10 +986,16 @@ class Auth_OpenID_GenericConsumer { } } - if ($to_match->claimed_id != $endpoint->claimed_id) { + // Fragments do not influence discovery, so we can't compare a + // claimed identifier with a fragment to discovered + // information. + list($defragged_claimed_id, $_) = + Auth_OpenID::urldefrag($to_match->claimed_id); + + if ($defragged_claimed_id != $endpoint->claimed_id) { return new Auth_OpenID_FailureResponse($endpoint, sprintf('Claimed ID does not match (different subjects!), ' . - 'Expected %s, got %s', $to_match->claimed_id, + 'Expected %s, got %s', $defragged_claimed_id, $endpoint->claimed_id)); } @@ -1045,42 +1051,58 @@ class Auth_OpenID_GenericConsumer { ($to_match->local_id !== null)) { return new Auth_OpenID_FailureResponse($endpoint, 'openid.identity is present without openid.claimed_id'); - } else if (($to_match->claimed_id !== null) && - ($to_match->local_id === null)) { + } + + if (($to_match->claimed_id !== null) && + ($to_match->local_id === null)) { return new Auth_OpenID_FailureResponse($endpoint, 'openid.claimed_id is present without openid.identity'); - } else if ($to_match->claimed_id === null) { + } + + if ($to_match->claimed_id === null) { // This is a response without identifiers, so there's // really no checking that we can do, so return an // endpoint that's for the specified `openid.op_endpoint' return Auth_OpenID_ServiceEndpoint::fromOPEndpointURL( $to_match->server_url); - } else if (!$endpoint) { + } + + // Fragments do not influence discovery, so we can't compare a + // claimed identifier with a fragment to discovered + // information. + list($defragged_claimed_id, $_) = + Auth_OpenID::urldefrag($to_match->claimed_id); + + if (!$endpoint) { // The claimed ID doesn't match, so we have to do // discovery again. This covers not using sessions, OP // identifier endpoints and responses that didn't match // the original request. // oidutil.log('No pre-discovered information supplied.') return $this->_discoverAndVerify($to_match); - } else if ($to_match->claimed_id != $endpoint->claimed_id) { + } else if ($defragged_claimed_id != $endpoint->claimed_id) { // oidutil.log('Mismatched pre-discovered session data. ' // 'Claimed ID in session=%s, in assertion=%s' % // (endpoint.claimed_id, to_match.claimed_id)) return $this->_discoverAndVerify($to_match); - } else { - // The claimed ID matches, so we use the endpoint that we - // discovered in initiation. This should be the most - // common case. - $result = $this->_verifyDiscoverySingle($endpoint, $to_match); + } - if (Auth_OpenID::isFailure($result)) { - return $result; - } + // The claimed ID matches, so we use the endpoint that we + // discovered in initiation. This should be the most common + // case. + $result = $this->_verifyDiscoverySingle($endpoint, $to_match); - return $endpoint; + if (Auth_OpenID::isFailure($result)) { + return $result; + } + + // The endpoint we return should have the claimed ID from the + // message we just verified, fragment and all. + if ($endpoint->claimed_id != $to_match->claimed_id) { + $endpoint->claimed_id = $to_match->claimed_id; } - // Never reached. + return $endpoint; } /** @@ -1092,6 +1114,7 @@ class Auth_OpenID_GenericConsumer { list($unused, $services) = call_user_func($this->discoverMethod, $to_match->claimed_id, $this->fetcher); + if (!$services) { return new Auth_OpenID_FailureResponse(null, sprintf("No OpenID information found at %s", diff --git a/Tests/Auth/OpenID/VerifyDisco.php b/Tests/Auth/OpenID/VerifyDisco.php index a3a6277..a95a4d3 100644 --- a/Tests/Auth/OpenID/VerifyDisco.php +++ b/Tests/Auth/OpenID/VerifyDisco.php @@ -150,6 +150,33 @@ class Tests_Auth_OpenID_VerifyDisco extends OpenIDTestMixin { $this->failUnlessProtocolError( $this->consumer->_verifyDiscoveryResults($msg, $endpoint)); } + + function test_openid2Fragment() + { + $claimed_id = "http://unittest.invalid/"; + $claimed_id_frag = $claimed_id . "#fragment"; + + $endpoint = new Auth_OpenID_ServiceEndpoint(); + $endpoint->local_id = 'my identity'; + $endpoint->claimed_id = $claimed_id; + $endpoint->server_url = 'Phone Home'; + $endpoint->type_uris = array(Auth_OpenID_TYPE_2_0); + + $msg = Auth_OpenID_Message::fromOpenIDArgs( + array('ns' => Auth_OpenID_OPENID2_NS, + 'identity' => $endpoint->local_id, + 'claimed_id' => $claimed_id_frag, + 'op_endpoint' => $endpoint->server_url)); + $result = $this->consumer->_verifyDiscoveryResults($msg, $endpoint); + + $this->assertEquals($result->local_id, $endpoint->local_id); + $this->assertEquals($result->server_url, $endpoint->server_url); + + $this->assertEquals($result->type_uris, $endpoint->type_uris); + + $this->assertEquals($result->claimed_id, $claimed_id_frag); + } + } // XXX: test the implementation of _discoverAndVerify @@ -178,7 +205,9 @@ class Tests_openID2NoEndpointDoesDisco extends Tests_Auth_OpenID_VerifyDisco { function test_openID2NoEndpointDoesDisco() { $op_endpoint = 'Phone Home'; - $sentinel = 'thing'; + $this->consumer->sentinel = new Auth_OpenID_ServiceEndpoint(); + $this->consumer->sentinel->claimed_id = 'monkeysoft'; + $msg = Auth_OpenID_Message::fromOpenIDArgs( array('ns' => Auth_OpenID_OPENID2_NS, 'identity' => 'sour grapes', @@ -199,6 +228,10 @@ class Tests_openID2MismatchedDoesDisco extends Tests_Auth_OpenID_VerifyDisco { $mismatched->identity = 'nothing special, but different'; $mismatched->local_id = 'green cheese'; + $sentinel = new Auth_OpenID_ServiceEndpoint(); + $sentinel->claimed_id = 'monkeysoft'; + $this->consumer->sentinel = $sentinel; + $op_endpoint = 'Phone Home'; $msg = Auth_OpenID_Message::fromOpenIDArgs( |