Page MenuHomePhorge
Files F15396918

D4929.id14134.diff
No OneTemporary
Actions

D4929.id14134.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

Mime Type
text/plain
Expires
Fri, Sep 20, 12:33 AM (6 h, 24 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
9464964
Default Alt Text
D4929.id14134.diff (21 KB)

Event Timeline