Adjust login url handling for authenticated users

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 
 - Improved error handling when a room requires an access code but none was provided ([#3035])
+- Login URL and external login redirect URLs now support authenticated users and honor the redirect query parameter ([#3078], [#3079])
 
 ### Fixed
 
@@ -781,6 +782,8 @@ You can find the changelog for older versions there [here](https://github.com/TH
 [#3035]: https://github.com/THM-Health/PILOS/pull/3035
 [#3039]: https://github.com/THM-Health/PILOS/issues/3039
 [#3040]: https://github.com/THM-Health/PILOS/pull/3040
+[#3078]: https://github.com/THM-Health/PILOS/issues/3078
+[#3079]: https://github.com/THM-Health/PILOS/pull/3079
 [unreleased]: https://github.com/THM-Health/PILOS/compare/v4.14.2...develop
 [v3.0.0]: https://github.com/THM-Health/PILOS/releases/tag/v3.0.0
 [v3.0.1]: https://github.com/THM-Health/PILOS/releases/tag/v3.0.1

app/Auth/OIDC/OIDCCallbackRequest.php

@@ -0,0 +1,26 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Auth\OIDC;
+
+use Illuminate\Contracts\Validation\Validator;
+use Illuminate\Foundation\Http\FormRequest;
+
+class OIDCCallbackRequest extends FormRequest
+{
+    public function rules()
+    {
+        return [
+            'code' => ['string'],
+            'state' => ['string'],
+            'error' => ['string'],
+            'error_description' => ['string'],
+        ];
+    }
+
+    protected function failedValidation(Validator $validator): void
+    {
+        abort(400);
+    }
+}

app/Auth/OIDC/OIDCController.php

@@ -15,16 +15,29 @@
 
 class OIDCController extends Controller
 {
+    protected const string REDIRECT_URL = '/external_login';
+
     public function __construct(protected OIDCProvider $provider)
     {
-        $this->middleware('guest');
+        $this->middleware('guest')->except('redirect');
     }
 
     /**
      * Redirect to the OpenID Provider for authentication with an optional redirect back to a specific URL
      */
-    public function redirect(Request $request)
+    public function redirect(OIDCRedirectRequest $request)
     {
+        if (Auth()->check()) {
+            $uri = Uri::of(self::REDIRECT_URL)->withQuery(['no_message' => true]);
+            if ($request->has('redirect')) {
+                return redirect($uri
+                    ->withQuery(['redirect' => $request->query('redirect')])
+                    ->value());
+            }
+
+            return redirect($uri->value());
+        }
+
         try {
             return $this->provider->redirect($request->query('redirect'));
         } catch (OpenIDConnectNetworkException $e) {
@@ -43,7 +56,7 @@ public function redirect(Request $request)
     /**
      * Handle Authorization Code Flow redirect back from the OpenID Provider with an Authorization Code
      */
-    public function callback(Request $request): RedirectResponse
+    public function callback(OIDCCallbackRequest $request): RedirectResponse
     {
         try {
             $user = $this->provider->login($request);
@@ -76,15 +89,13 @@ public function callback(Request $request): RedirectResponse
         $user->last_login = now();
         $user->save();
 
-        $url = '/external_login';
-
         if (session()->has('redirect_url')) {
-            return redirect(Uri::of($url)
+            return redirect(Uri::of(self::REDIRECT_URL)
                 ->withQuery(['redirect' => session()->get('redirect_url')])
                 ->value());
         }
 
-        return redirect($url);
+        return redirect(self::REDIRECT_URL);
     }
 
     /**

app/Auth/OIDC/OIDCProvider.php

@@ -76,10 +76,7 @@ public function redirect($redirect = null)
      */
     public function login(Request $request): User
     {
-        if (! $this->openIDConnectClient->authenticate($request)) {
-            // Response is missing the code parameters
-            throw new OpenIDConnectCodeMissingException("Response is missing 'code' parameter.");
-        }
+        $this->openIDConnectClient->authenticate($request);
 
         $claims = $this->openIDConnectClient->getVerifiedClaims();
 

app/Auth/OIDC/OIDCRedirectRequest.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Auth\OIDC;
+
+use Illuminate\Contracts\Validation\Validator;
+use Illuminate\Foundation\Http\FormRequest;
+
+class OIDCRedirectRequest extends FormRequest
+{
+    public function rules()
+    {
+        return [
+            'redirect' => ['string'],
+        ];
+    }
+
+    protected function failedValidation(Validator $validator): void
+    {
+        abort(400);
+    }
+}

app/Auth/OIDC/OpenIDConnectClient.php

@@ -181,21 +181,19 @@ public function __construct(private string $providerUrl, private string $clientI
      * Authenticate the user with the OpenID Connect provider using the authorization code
      *
      * @param  Request  $request  The request object containing the authorization code and state
-     * @return bool Returns true if authentication is successful, false if the code is missing
      *
-     * @throws OpenIDConnectClientException
-     * @throws OpenIDConnectNetworkException
      * @throws ConnectionException
+     * @throws InvalidClaimException
+     * @throws JsonException
+     * @throws MissingMandatoryClaimException
      * @throws OpenIDConnectClientException
+     * @throws OpenIDConnectCodeMissingException
      * @throws OpenIDConnectNetworkException
-     * @throws RequestException
-     * @throws OpenIDConnectValidationException
      * @throws OpenIDConnectProviderException
-     * @throws JsonException
-     * @throws InvalidClaimException
-     * @throws MissingMandatoryClaimException
+     * @throws OpenIDConnectValidationException
+     * @throws RequestException
      */
-    public function authenticate(Request $request): bool
+    public function authenticate(Request $request): void
     {
         // Do a preemptive check to see if the provider has thrown an error from a previous redirect
         if ($request->has('error')) {
@@ -206,7 +204,7 @@ public function authenticate(Request $request): bool
         // If the authorization code is missing, the authentication has failed
         // User might have called the authentication URL directly
         if (! $request->has('code')) {
-            return false;
+            throw new OpenIDConnectCodeMissingException("Response is missing 'code' parameter.");
         }
 
         // Check OpenID Connect session
@@ -264,7 +262,7 @@ public function authenticate(Request $request): bool
         $this->verifiedClaims = $claims;
 
         // Success!
-        return true;
+
     }
 
     /**

app/Auth/Shibboleth/ShibbolethCallbackRequest.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Auth\Shibboleth;
+
+use Illuminate\Contracts\Validation\Validator;
+use Illuminate\Foundation\Http\FormRequest;
+
+class ShibbolethCallbackRequest extends FormRequest
+{
+    public function rules()
+    {
+        return [
+            'redirect' => ['string'],
+        ];
+    }
+
+    protected function failedValidation(Validator $validator): void
+    {
+        abort(400);
+    }
+}

app/Auth/Shibboleth/ShibbolethController.php

@@ -9,19 +9,33 @@
 use App\Prometheus\Counter;
 use Illuminate\Http\Request;
 use Illuminate\Support\Facades\Log;
+use Illuminate\Support\Uri;
 
 class ShibbolethController extends Controller
 {
+    protected const string REDIRECT_URL = '/external_login';
+
     public function __construct(protected ShibbolethProvider $provider)
     {
-        $this->middleware('guest')->except(['logout']);
+        $this->middleware('guest')->except(['logout', 'redirect']);
     }
 
     /**
      * Redirect to the Shibboleth for authentication with an optional redirect back to a specific URL
      */
-    public function redirect(Request $request)
+    public function redirect(ShibbolethRedirectRequest $request)
     {
+        if (Auth()->check()) {
+            $uri = Uri::of(self::REDIRECT_URL)->withQuery(['no_message' => true]);
+            if ($request->has('redirect')) {
+                return redirect($uri
+                    ->withQuery(['redirect' => $request->query('redirect')])
+                    ->value());
+            }
+
+            return redirect($uri->value());
+        }
+
         return $this->provider->redirect($request->query('redirect'));
     }
 
@@ -44,7 +58,7 @@ public function logout(Request $request)
     /**
      * Request to login with shibboleth, route is protected by mod-shibb of the reverse proxy
      */
-    public function callback(Request $request)
+    public function callback(ShibbolethCallbackRequest $request)
     {
         try {
             $user = $this->provider->login($request);
@@ -65,9 +79,12 @@ public function callback(Request $request)
         $user->save();
 
         // Redirect to the external login page in the frontend, optionally with a redirect back to a specific URL
-        $url = '/external_login';
-        $redirectUrl = $request->query('redirect');
+        if ($request->has('redirect')) {
+            return redirect(Uri::of(self::REDIRECT_URL)
+                ->withQuery(['redirect' => $request->query('redirect')])
+                ->value());
+        }
 
-        return redirect($redirectUrl ? ($url.'?redirect='.urlencode($redirectUrl)) : $url);
+        return redirect(self::REDIRECT_URL);
     }
 }

app/Auth/Shibboleth/ShibbolethRedirectRequest.php

@@ -0,0 +1,23 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Auth\Shibboleth;
+
+use Illuminate\Contracts\Validation\Validator;
+use Illuminate\Foundation\Http\FormRequest;
+
+class ShibbolethRedirectRequest extends FormRequest
+{
+    public function rules()
+    {
+        return [
+            'redirect' => ['string'],
+        ];
+    }
+
+    protected function failedValidation(Validator $validator): void
+    {
+        abort(400);
+    }
+}

resources/js/router.js

@@ -62,7 +62,6 @@ export const routes = [
     path: "/login",
     name: "login",
     component: Login,
-    meta: { guestsOnly: true },
   },
   {
     path: "/external_login",
@@ -71,6 +70,7 @@ export const routes = [
     props: (route) => {
       return {
         error: route.query.error,
+        noMessage: route.query.no_message === "1",
       };
     },
   },

resources/js/views/ExternalLogin.vue

@@ -50,7 +50,7 @@
 </template>
 
 <script setup>
-import { onMounted } from "vue";
+import { computed, onMounted } from "vue";
 import { useToast } from "../composables/useToast.js";
 import { useRoute, useRouter } from "vue-router";
 import { useI18n } from "vue-i18n";
@@ -60,22 +60,35 @@ const props = defineProps({
     type: String,
     default: null,
   },
+  noMessage: {
+    type: Boolean,
+    default: false,
+  },
 });
 
 const toast = useToast();
 const { t } = useI18n();
 const router = useRouter();
 const route = useRoute();
 
+const normalizedRedirect = computed(() => {
+  const value = route.query.redirect;
+  const redirectValue = Array.isArray(value) ? value[0] : value;
+  return redirectValue || undefined;
+});
+
 onMounted(() => {
   // Successfully login via external provider
   if (!props.error) {
-    // show toast message
-    toast.success(t("auth.flash.login"));
+    if (!props.noMessage) {
+      // show toast message
+      toast.success(t("auth.flash.login"));
+    }
+
     // check if user should be redirected back after login,
     // otherwise redirect to own rooms (dashboard)
-    if (route.query.redirect !== undefined) {
-      router.push(route.query.redirect);
+    if (normalizedRedirect.value !== undefined) {
+      router.push(normalizedRedirect.value);
     } else {
       router.push({ name: "rooms.index" });
     }