Page Menu
Home
Phorge
Search
Configure Global Search
Log In
Files
F15416379
D4929.id14136.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Flag For Later
Award Token
Size
21 KB
Referenced Files
None
Subscribers
None
D4929.id14136.diff
View Options
diff --git a/src/app/Http/Controllers/API/AuthController.php b/src/app/Http/Controllers/API/AuthController.php
--- a/src/app/Http/Controllers/API/AuthController.php
+++ b/src/app/Http/Controllers/API/AuthController.php
@@ -2,10 +2,11 @@
namespace App\Http\Controllers\API;
+use App\Auth\PassportClient;
use App\Http\Controllers\Controller;
use App\User;
use Illuminate\Http\Request;
-use Illuminate\Support\Facades\Auth;
+use Illuminate\Support\Facades\Cache;
use Illuminate\Support\Facades\Validator;
use Laravel\Passport\TokenRepository;
use Laravel\Passport\RefreshTokenRepository;
@@ -22,7 +23,7 @@
*/
public function info()
{
- $user = Auth::guard()->user();
+ $user = $this->guard()->user();
if (!empty(request()->input('refresh'))) {
return $this->refreshAndRespond(request(), $user);
@@ -107,11 +108,15 @@
*/
public function oauthApprove(ServerRequestInterface $psrRequest, Request $request, AuthorizationServer $server)
{
- if ($request->response_type != 'code') {
- return self::errorResponse(422, self::trans('validation.invalidvalueof', ['attribute' => 'response_type']));
- }
+ $clientId = $request->input('client_id');
+ $user = $this->guard()->user();
+ $cacheKey = "oauth-seen-{$user->id}-{$clientId}";
try {
+ if ($request->response_type != 'code') {
+ throw new \Exception('Invalid response_type');
+ }
+
// OpenID handler reads parameters from the request query string (GET)
$request->query->replace($request->input());
@@ -120,7 +125,14 @@
$authRequest = $server->validateAuthorizationRequest($psrRequest);
- $user = Auth::guard()->user();
+ // Check if the client was approved before (in last x days)
+ if ($clientId && $request->ifSeen) {
+ $client = PassportClient::find($clientId);
+
+ if ($client && !Cache::has($cacheKey)) {
+ throw new \Exception('Not seen yet');
+ }
+ }
// TODO I'm not sure if we should still execute this to deny the request
$authRequest->setUser(new \Laravel\Passport\Bridge\User($user->getAuthIdentifier()));
@@ -128,12 +140,45 @@
// This will generate a 302 redirect to the redirect_uri with the generated authorization code
$response = $server->completeAuthorizationRequest($authRequest, new Psr7Response());
+
+ // Remember the approval for x days.
+ // In this time we'll not show the UI form and we'll redirect automatically
+ // TODO: If we wanted to give users ability to remove this "approved" state for a client,
+ // we would have to store these records in SQL table. It would become handy especially
+ // if we give users possibility to register external OAuth apps.
+ Cache::put($cacheKey, 1, now()->addDays(14));
} catch (\League\OAuth2\Server\Exception\OAuthServerException $e) {
// Note: We don't want 401 or 400 codes here, use 422 which is used in our API
$code = $e->getHttpStatusCode();
- return self::errorResponse($code < 500 ? 422 : 500, $e->getMessage());
+ $response = $e->getPayload();
+ $response['redirectUrl'] = !empty($client) ? $client->redirect : $request->input('redirect_uri');
+
+ return self::errorResponse($code < 500 ? 422 : 500, $e->getMessage(), $response);
} catch (\Exception $e) {
- return self::errorResponse(422, self::trans('auth.error.invalidrequest'));
+ if (!empty($client)) {
+ $scopes = preg_split('/\s+/', (string) $request->input('scope'));
+
+ $claims = [];
+ foreach (array_intersect($scopes, $client->allowed_scopes) as $claim) {
+ $claims[$claim] = self::trans("auth.claim.{$claim}");
+ }
+
+ return response()->json([
+ 'status' => 'prompt',
+ 'client' => [
+ 'name' => $client->name,
+ 'url' => $client->redirect,
+ 'claims' => $claims,
+ ],
+ ]);
+ }
+
+ $response = [
+ 'error' => $e->getMessage() == 'Invalid response_type' ? 'unsupported_response_type' : 'server_error',
+ 'redirectUrl' => $request->input('redirect_uri'),
+ ];
+
+ return self::errorResponse(422, self::trans('auth.error.invalidrequest'), $response);
}
return response()->json([
@@ -149,7 +194,7 @@
*/
public function oauthUserInfo()
{
- $user = Auth::guard()->user();
+ $user = $this->guard()->user();
$response = [
// Per OIDC spec. 'sub' must be always returned
@@ -196,8 +241,7 @@
*/
public function logout()
{
- $tokenId = Auth::user()->token()->id;
-
+ $tokenId = $this->guard()->user()->token()->id;
$tokenRepository = app(TokenRepository::class);
$refreshTokenRepository = app(RefreshTokenRepository::class);
diff --git a/src/resources/lang/en/auth.php b/src/resources/lang/en/auth.php
--- a/src/resources/lang/en/auth.php
+++ b/src/resources/lang/en/auth.php
@@ -18,11 +18,14 @@
'throttle' => 'Too many login attempts. Please try again in :seconds seconds.',
'logoutsuccess' => 'Successfully logged out.',
+ 'claim.unknown' => "Unknown claim",
+ 'claim.email' => "See your email address",
+ 'claim.auth.token' => "Have read and write access to all your data",
+
'error.password' => "Invalid password",
'error.invalidrequest' => "Invalid authorization request.",
'error.geolocation' => "Country code mismatch",
'error.notfound' => "User not found",
'error.2fa' => "Second factor failure",
'error.2fa-generic' => "Second factor failure",
-
];
diff --git a/src/resources/lang/en/ui.php b/src/resources/lang/en/ui.php
--- a/src/resources/lang/en/ui.php
+++ b/src/resources/lang/en/ui.php
@@ -12,6 +12,14 @@
'faq' => "FAQ",
],
+ 'auth' => [
+ 'allow' => "Allow access",
+ 'authorize-header' => "The application would like to:",
+ 'authorize-title' => "{name} is asking for permission",
+ 'authorize-footer' => "On any action you will be redirected to {url}",
+ 'deny' => "No, thanks",
+ ],
+
'btn' => [
'add' => "Add",
'accept' => "Accept",
diff --git a/src/resources/vue/Authorize.vue b/src/resources/vue/Authorize.vue
--- a/src/resources/vue/Authorize.vue
+++ b/src/resources/vue/Authorize.vue
@@ -1,31 +1,107 @@
<template>
- <div class="container" id="auth-container">
+ <div class="container d-flex flex-column align-items-center justify-content-center" id="auth-container">
+ <div v-if="client" id="auth-form" class="card col-sm-8 col-lg-6">
+ <div class="card-body p-4 text-center">
+ <h1 class="card-title mb-3">{{ $t('auth.authorize-title', client) }}</h1>
+ <div class="card-text m-2 mb-0">
+ <h6 id="auth-email" class="text-secondary mb-4">
+ <svg-icon icon="user" class="me-2"></svg-icon>{{ client.email }}
+ </h6>
+ <p id="auth-header">
+ {{ $t('auth.authorize-header', client) }}
+ </p>
+ <p>
+ <ul id="auth-claims" class="list-group text-start">
+ <li class="list-group-item" v-for="(item, idx) in client.claims" :key="idx">{{ item }}</li>
+ </ul>
+ </p>
+ <small id="auth-footer" class="text-secondary">
+ {{ $t('auth.authorize-footer', client) }}
+ </small>
+ <p class="mt-4">
+ <btn class="btn-success" icon="check" @click="allow">{{ $t('auth.allow') }}</btn>
+ <btn class="btn-danger ms-5" icon="xmark" @click="deny">{{ $t('auth.deny') }}</btn>
+ </p>
+ </div>
+ </div>
+ </div>
</div>
</template>
<script>
+ import { library } from '@fortawesome/fontawesome-svg-core'
+
+ library.add(
+ require('@fortawesome/free-solid-svg-icons/faXmark').definition,
+ )
+
export default {
+ data() {
+ return {
+ client: null,
+ }
+ },
created() {
- // Just auto approve for now
- // If we wanted we could use this page to list what is being authorized,
- // and allow the user to approve/reject.
- // Note that in case of SSO it is also expected that there's no user interaction at all,
- // isn't it? Maybe we should show the details once per client_id+user_id combination.
- this.submitApproval()
+ this.submit(true)
},
methods: {
- submitApproval() {
+ allow() {
+ this.submit()
+ },
+ deny() {
+ if (this.client.url) {
+ this.redirect(this.client.url, { error: 'access_denied', state: this.$route.query.state })
+ }
+ },
+ redirect(url, params) {
+ // Merge additional parameters with the URL (that can already contain a search query)
+ if (params) {
+ url = URL.parse(url)
+
+ const search = new URLSearchParams(url.searchParams)
+ for (const [k, v] of Object.entries(params)) {
+ search.set(k, v)
+ }
+
+ url.search = search
+ }
+
+ // Display loading widget, redirecting may take a while
+ this.$root.startLoading(['#auth-container', { small: false, text: this.$t('msg.redirecting') }])
+
+ // Follow the redirect to the external page
+ window.location.href = url
+ },
+ submit(ifSeen = false) {
let props = ['client_id', 'redirect_uri', 'state', 'nonce', 'scope', 'response_type', 'response_mode']
let post = this.$root.pick(this.$route.query, props)
+ let redirect = null
+
+ post.ifSeen = ifSeen
axios.post('/api/oauth/approve', post, { loading: true })
.then(response => {
- // Display loading widget, redirecting may take a while
- this.$root.startLoading(['#auth-container', { small: false, text: this.$t('msg.redirecting') }])
- // Follow the redirect to the external page
- window.location.href = response.data.redirectUrl
+ if (response.data.status == 'prompt') {
+ // Display the form with Allow/Deny buttons
+ this.client = response.data.client
+ this.client.email = this.$root.authInfo.email
+ } else {
+ // Redirect to the external page
+ redirect = response.data
+ }
+ })
+ .catch(error => {
+ if (!(redirect = error.response.data)) {
+ this.$root.errorHandler(error)
+ }
+ })
+ .finally(() => {
+ if (redirect && redirect.redirectUrl) {
+ let params = this.$root.pick(redirect, ['error', 'error_description'])
+ params.state = this.$route.query.state
+ this.redirect(redirect.redirectUrl, params)
+ }
})
- .catch(this.$root.errorHandler)
}
}
}
diff --git a/src/tests/Browser/AuthorizeTest.php b/src/tests/Browser/AuthorizeTest.php
--- a/src/tests/Browser/AuthorizeTest.php
+++ b/src/tests/Browser/AuthorizeTest.php
@@ -2,6 +2,7 @@
namespace Tests\Browser;
+use Illuminate\Support\Facades\Cache;
use Tests\Browser;
use Tests\Browser\Components\Toast;
use Tests\Browser\Pages\Home;
@@ -26,11 +27,11 @@
'name' => 'Test',
'secret' => '123',
'provider' => 'users',
- 'redirect' => 'https://kolab.org',
+ 'redirect' => \App\Utils::serviceUrl('support'),
'personal_access_client' => 0,
'password_client' => 0,
'revoked' => false,
- 'allowed_scopes' => ['email'],
+ 'allowed_scopes' => ['email', 'auth.token'],
]
);
}
@@ -50,31 +51,57 @@
*/
public function testAuthorize(): void
{
+ $user = $this->getTestUser('john@kolab.org');
+
$url = '/oauth/authorize?' . http_build_query([
'client_id' => $this->client->id,
'response_type' => 'code',
- 'scope' => 'email',
+ 'scope' => 'email auth.token',
'state' => 'state',
+ 'redirect_uri' => $this->client->redirect,
]);
- $this->browse(function (Browser $browser) use ($url) {
- $redirect_check = "window.location.host == 'kolab.org'"
- . " && window.location.search.match(/^\?code=[a-f0-9]+&state=state/)";
+ Cache::forget("oauth-seen-{$user->id}-{$this->client->id}");
- // Unauthenticated user
+ $this->browse(function (Browser $browser) use ($url, $user) {
+ // Visit the page and expect logon form, then log in
$browser->visit($url)
->on(new Home())
- ->submitLogon('john@kolab.org', 'simple123')
- ->waitUntil($redirect_check);
+ ->submitLogon($user->email, 'simple123');
+
+ // Expect the claims form
+ $browser->waitFor('#auth-form')
+ ->assertSeeIn('#auth-form h1', "Test is asking for permission")
+ ->assertSeeIn('#auth-email', $user->email)
+ ->assertVisible('#auth-header')
+ ->assertElementsCount('#auth-claims li', 2)
+ ->assertSeeIn('#auth-claims li:nth-child(1)', "See your email address")
+ ->assertSeeIn('#auth-claims li:nth-child(2)', "Have read and write access to")
+ ->assertSeeIn('#auth-footer', $this->client->redirect)
+ ->assertSeeIn('#auth-form button.btn-success', 'Allow access')
+ ->assertSeeIn('#auth-form button.btn-danger', 'No, thanks');
+
+ // Click the "No, thanks" button
+ $browser->click('#auth-form button.btn-danger')
+ ->waitForLocation('/support')
+ ->assertScript("location.search.match(/^\?error=access_denied&state=state/) !== null");
+
+ // Visit the page again and click the "Allow access" button
+ $browser->visit($url)
+ ->waitFor('#auth-form button.btn-success')
+ ->click('#auth-form button.btn-success')
+ ->waitForLocation('/support')
+ ->assertScript("location.search.match(/^\?code=[a-f0-9]+&state=state/) !== null");
- // Authenticated user
+ // Visit the page and expect an immediate redirect
$browser->visit($url)
- ->waitUntil($redirect_check);
+ ->waitForLocation('/support')
+ ->assertScript("location.search.match(/^\?code=[a-f0-9]+&state=state/) !== null");
// Error handling (invalid response_type)
- $browser->visit('oauth/authorize?response_type=invalid')
- ->assertErrorPage(422)
- ->assertToast(Toast::TYPE_ERROR, 'Invalid value of request property: response_type.');
+ $browser->visit(str_replace('response_type=code', 'response_type=invalid', $url))
+ ->waitForLocation('/support')
+ ->assertScript("location.search.match(/^\?error=unsupported_response_type&state=state/) !== null");
});
}
}
diff --git a/src/tests/BrowserAddonTrait.php b/src/tests/BrowserAddonTrait.php
--- a/src/tests/BrowserAddonTrait.php
+++ b/src/tests/BrowserAddonTrait.php
@@ -55,7 +55,7 @@
*/
protected function driver()
{
- static::startChromeDriver();
+ static::startChromeDriver(['--port=9515']);
$options = (new ChromeOptions())->addArguments([
'--lang=en_US',
diff --git a/src/tests/Feature/Controller/AuthTest.php b/src/tests/Feature/Controller/AuthTest.php
--- a/src/tests/Feature/Controller/AuthTest.php
+++ b/src/tests/Feature/Controller/AuthTest.php
@@ -334,7 +334,8 @@
$json = $response->json();
$this->assertSame('error', $json['status']);
- $this->assertSame('Invalid value of request property: response_type.', $json['message']);
+ $this->assertSame('unsupported_response_type', $json['error']);
+ $this->assertSame('Invalid authorization request.', $json['message']);
// Request authenticated, invalid POST data
$post = [
@@ -350,6 +351,7 @@
$json = $response->json();
$this->assertSame('error', $json['status']);
+ $this->assertSame('invalid_client', $json['error']);
$this->assertSame('Client authentication failed', $json['message']);
$client = \App\Auth\PassportClient::find(\config('auth.synapse.client_id'));
@@ -364,6 +366,7 @@
$json = $response->json();
$this->assertSame('error', $json['status']);
+ $this->assertSame('invalid_scope', $json['error']);
$this->assertSame('The requested scope is invalid, unknown, or malformed', $json['message']);
// Request authenticated, valid POST data
@@ -465,11 +468,72 @@
$response->assertStatus(403);
}
+ /**
+ * Test Oauth approve end-point in ifSeen mode
+ */
+ public function testOAuthApprovePrompt(): void
+ {
+ // HTTP_HOST is not set in tests for some reason, but it's required down the line
+ $host = parse_url(\App\Utils::serviceUrl('/'), \PHP_URL_HOST);
+ $_SERVER['HTTP_HOST'] = $host;
+
+ $user = $this->getTestUser('UsersControllerTest1@userscontroller.com');
+ $client = \App\Auth\PassportClient::find(\config('auth.sso.client_id'));
+
+ $post = [
+ 'client_id' => $client->id,
+ 'response_type' => 'code',
+ 'scope' => 'openid email auth.token',
+ 'state' => 'state',
+ 'nonce' => 'nonce',
+ 'ifSeen' => '1',
+ ];
+
+ $response = $this->actingAs($user)->post("api/oauth/approve", $post);
+ $response->assertStatus(200);
+
+ $json = $response->json();
+
+ $claims = [
+ 'email' => 'See your email address',
+ 'auth.token' => 'Have read and write access to all your data',
+ ];
+
+ $this->assertSame('prompt', $json['status']);
+ $this->assertSame($client->name, $json['client']['name']);
+ $this->assertSame($client->redirect, $json['client']['url']);
+ $this->assertSame($claims, $json['client']['claims']);
+
+ // Approve the request
+ $post['ifSeen'] = 0;
+ $response = $this->actingAs($user)->post("api/oauth/approve", $post);
+ $response->assertStatus(200);
+
+ $json = $response->json();
+
+ $this->assertSame('success', $json['status']);
+ $this->assertTrue(!empty($json['redirectUrl']));
+
+ // Second request with ifSeen=1 should succeed with the code
+ $post['ifSeen'] = 1;
+ $response = $this->actingAs($user)->post("api/oauth/approve", $post);
+ $response->assertStatus(200);
+
+ $json = $response->json();
+
+ $this->assertSame('success', $json['status']);
+ $this->assertTrue(!empty($json['redirectUrl']));
+ }
+
/**
* Test OpenID-Connect Authorization Code Flow
*/
public function testOIDCAuthorizationCodeFlow(): void
{
+ // HTTP_HOST is not set in tests for some reason, but it's required down the line
+ $host = parse_url(\App\Utils::serviceUrl('/'), \PHP_URL_HOST);
+ $_SERVER['HTTP_HOST'] = $host;
+
$user = $this->getTestUser('john@kolab.org');
$client = \App\Auth\PassportClient::find(\config('auth.sso.client_id'));
diff --git a/src/tests/Feature/Controller/WellKnownTest.php b/src/tests/Feature/Controller/WellKnownTest.php
--- a/src/tests/Feature/Controller/WellKnownTest.php
+++ b/src/tests/Feature/Controller/WellKnownTest.php
@@ -13,6 +13,10 @@
{
$href = \App\Utils::serviceUrl('/');
+ // HTTP_HOST is not set in tests for some reason, but it's required down the line
+ $host = parse_url($href, \PHP_URL_HOST);
+ $_SERVER['HTTP_HOST'] = $host;
+
$response = $this->get('.well-known/openid-configuration');
$response->assertStatus(200)
->assertJson([
diff --git a/src/tests/TestCaseDusk.php b/src/tests/TestCaseDusk.php
--- a/src/tests/TestCaseDusk.php
+++ b/src/tests/TestCaseDusk.php
@@ -23,6 +23,7 @@
// For troubleshooting
// '--verbose',
// '--log-path=/tmp/chromedriver.log',
+ '--port=9515',
]);
}
@@ -81,7 +82,7 @@
$options->setExperimentalOption('mobileEmulation', ['userAgent' => $ua]);
$options->addArguments(['--window-size=375,667']);
} elseif (getenv('TESTS_MODE') == 'tablet') {
- // Fake User-Agent string for mobile mode
+ // Fake User-Agent string for tablet mode
$ua = 'Mozilla/5.0 (Linux; Android 6.0.1; vivo 1603 Build/MMB29M) AppleWebKit/537.36 '
. ' (KHTML, like Gecko) Chrome/58.0.3029.83 Mobile Safari/537.36';
$options->setExperimentalOption('mobileEmulation', ['userAgent' => $ua]);
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Fri, Sep 20, 11:33 PM (20 h, 30 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
9464964
Default Alt Text
D4929.id14136.diff (21 KB)
Attached To
Mode
D4929: OAuth authorization UI, proper error handling
Attached
Detach File
Event Timeline
Log In to Comment