Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Implement basic OIDC core server handling #12567

Closed
wants to merge 3 commits into from
Closed

Implement basic OIDC core server handling #12567

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f9d3840
Implement basic OIDC core server handling
Nov 21, 2018
fd3983b
Merge remote-tracking branch 'upstream/master' into oidc
Feb 15, 2019
21eef41
extract idToken generation into own function
Feb 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 46 additions & 1 deletion apps/oauth2/lib/Controller/OauthApiController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IRequest;
use OCP\IUserManager;
use OCP\Security\ICrypto;
use OCP\Security\ISecureRandom;

Expand All @@ -53,6 +54,8 @@ class OauthApiController extends Controller {
private $time;
/** @var Throttler */
private $throttler;
/** @var IUserManager */
private $userManager;

public function __construct(string $appName,
IRequest $request,
Expand All @@ -62,7 +65,8 @@ public function __construct(string $appName,
TokenProvider $tokenProvider,
ISecureRandom $secureRandom,
ITimeFactory $time,
Throttler $throttler) {
Throttler $throttler,
IUserManager $userManager) {
parent::__construct($appName, $request);
$this->crypto = $crypto;
$this->accessTokenMapper = $accessTokenMapper;
Expand All @@ -71,6 +75,7 @@ public function __construct(string $appName,
$this->secureRandom = $secureRandom;
$this->time = $time;
$this->throttler = $throttler;
$this->userManager = $userManager;
}

/**
Expand Down Expand Up @@ -162,13 +167,53 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client

$this->throttler->resetDelay($this->request->getRemoteAddress(), 'login', ['user' => $appToken->getUID()]);

// The id token needs to be correctly build as JWT. Taken from https://dev.to/robdwaller/how-to-create-a-json-web-token-using-php-3gml
tisoft marked this conversation as resolved.
Show resolved Hide resolved

// Create token header as a JSON string
$header = json_encode(['typ' => 'JWT', 'alg' => 'HS256']);

// We need the user to fill in name and email in the id_token
$user = $this->userManager->get($appToken->getUID());

// Create token payload as a JSON string
$payload = json_encode([
tisoft marked this conversation as resolved.
Show resolved Hide resolved
// required for OIDC
'iss' => \OC::$server->getURLGenerator()->getBaseUrl(),
tisoft marked this conversation as resolved.
Show resolved Hide resolved
'sub' => $appToken->getUID(),
'aud' => $client_id,
'exp' => $appToken->getExpires(),
'iat' => $this->time->getTime(),
'auth_time' => $this->time->getTime(),

// optional, can be requested by claims, we don't support requesting claims as of now, so we just send them always
'email' => $user->getEMailAddress(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a problem if those are empty?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, they can be left empty.

'name' => $user->getDisplayName(),

]);

// Encode Header to Base64Url String
$base64UrlHeader = str_replace(['+', '/', '='], ['-', '_', ''], base64_encode($header));

// Encode Payload to Base64Url String
$base64UrlPayload = str_replace(['+', '/', '='], ['-', '_', ''], base64_encode($payload));

// Create Signature Hash
$signature = hash_hmac('sha256', $base64UrlHeader . "." . $base64UrlPayload, $client->getSecret(), true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the signature is required it seems. But how can it every be validated?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The OpenID Connect client will do the same hash calculation on his side. Since the client knows the oauth client secret he can do that. That way the signature can be verified.


// Encode Signature to Base64Url String
$base64UrlSignature = str_replace(['+', '/', '='], ['-', '_', ''], base64_encode($signature));

// Create JWT
$jwt = $base64UrlHeader . "." . $base64UrlPayload . "." . $base64UrlSignature;

return new JSONResponse(
[
'access_token' => $newToken,
'token_type' => 'Bearer',
'expires_in' => 3600,
'refresh_token' => $newCode,
'user_id' => $appToken->getUID(),
'id_token' => $jwt,
]
);
}
Expand Down
79 changes: 78 additions & 1 deletion apps/oauth2/tests/Controller/OauthApiControllerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\IRequest;
use OCP\IUser;
use OCP\IUserManager;
use OCP\Security\ICrypto;
use OCP\Security\ISecureRandom;
use Test\TestCase;
Expand All @@ -58,6 +60,8 @@ class OauthApiControllerTest extends TestCase {
private $time;
/** @var Throttler|\PHPUnit_Framework_MockObject_MockObject */
private $throttler;
/** @var IUserManager|\PHPUnit_Framework_MockObject_MockObject */
private $userManager;
/** @var OauthApiController */
private $oauthApiController;

Expand All @@ -72,6 +76,7 @@ public function setUp() {
$this->secureRandom = $this->createMock(ISecureRandom::class);
$this->time = $this->createMock(ITimeFactory::class);
$this->throttler = $this->createMock(Throttler::class);
$this->userManager = $this->createMock(IUserManager::class);

$this->oauthApiController = new OauthApiController(
'oauth2',
Expand All @@ -82,7 +87,8 @@ public function setUp() {
$this->tokenProvider,
$this->secureRandom,
$this->time,
$this->throttler
$this->throttler,
$this->userManager
);
}

Expand Down Expand Up @@ -287,6 +293,16 @@ public function testGetTokenValidAppToken() {
'expires_in' => 3600,
'refresh_token' => 'random128',
'user_id' => 'userId',
'id_token' => $this->encodeJWT(json_encode([
'iss' => 'http://localhost',
'sub' => 'userId',
'aud' => 'clientId',
'exp' => 4600,
'iat' => 1000,
'auth_time' => 1000,
'email' => null,
'name' => null
]), 'clientSecret')
]);

$this->request->method('getRemoteAddress')
Expand All @@ -300,6 +316,13 @@ public function testGetTokenValidAppToken() {
['user' => 'userId']
);

$user = $this->createMock(IUser::class);;

$this->userManager->expects($this->once())
->method('get')
->with('userId')
->willReturn($user);

$this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret'));
}

Expand Down Expand Up @@ -379,6 +402,16 @@ public function testGetTokenValidAppTokenBasicAuth() {
'expires_in' => 3600,
'refresh_token' => 'random128',
'user_id' => 'userId',
'id_token' => $this->encodeJWT(json_encode([
'iss' => 'http://localhost',
'sub' => 'userId',
'aud' => 'clientId',
'exp' => 4600,
'iat' => 1000,
'auth_time' => 1000,
'email' => null,
'name' => null
]), 'clientSecret'),
]);

$this->request->server['PHP_AUTH_USER'] = 'clientId';
Expand All @@ -395,6 +428,13 @@ public function testGetTokenValidAppTokenBasicAuth() {
['user' => 'userId']
);

$user = $this->createMock(IUser::class);;

$this->userManager->expects($this->once())
->method('get')
->with('userId')
->willReturn($user);

$this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', null, null));
}

Expand Down Expand Up @@ -474,6 +514,16 @@ public function testGetTokenExpiredAppToken() {
'expires_in' => 3600,
'refresh_token' => 'random128',
'user_id' => 'userId',
'id_token' => $this->encodeJWT(json_encode([
'iss' => 'http://localhost',
'sub' => 'userId',
'aud' => 'clientId',
'exp' => 4600,
'iat' => 1000,
'auth_time' => 1000,
'email' => null,
'name' => null
]), 'clientSecret'),
]);

$this->request->method('getRemoteAddress')
Expand All @@ -487,6 +537,33 @@ public function testGetTokenExpiredAppToken() {
['user' => 'userId']
);

$user = $this->createMock(IUser::class);;

$this->userManager->expects($this->once())
->method('get')
->with('userId')
->willReturn($user);

$this->assertEquals($expected, $this->oauthApiController->getToken('refresh_token', null, 'validrefresh', 'clientId', 'clientSecret'));
}

private function encodeJWT($payload, $secret) {
// Create token header as a JSON string
$header = json_encode(['typ' => 'JWT', 'alg' => 'HS256']);

// Encode Header to Base64Url String
$base64UrlHeader = str_replace(['+', '/', '='], ['-', '_', ''], base64_encode($header));

// Encode Payload to Base64Url String
$base64UrlPayload = str_replace(['+', '/', '='], ['-', '_', ''], base64_encode($payload));

// Create Signature Hash
$signature = hash_hmac('sha256', $base64UrlHeader . "." . $base64UrlPayload, $secret, true);

// Encode Signature to Base64Url String
$base64UrlSignature = str_replace(['+', '/', '='], ['-', '_', ''], base64_encode($signature));

// Create JWT
return $base64UrlHeader . "." . $base64UrlPayload . "." . $base64UrlSignature;
}
}