Adding well-known configuration and JWKS routes in Laravel #16

moufmouf · 2024-01-18T16:56:14Z

When using Laravel with Passport, we have enough knowledge of the environment
to be able to provide 2 useful routes our of the box:

a discovery endpoint at /.well-known/openid-configuration.
a JWKS endpoint at /oauth/jwks.

This greatly eases integrating with Laravel since clients can now
use auto-discovery to integrate with Laravel + PassPort + OpenID.

This PR has been successfully tested using the Javascript openid-client package.
This PR is based on top of #13

mrmstn · 2024-02-05T17:06:36Z

I would be very interested in this PR, is there something left to do @ronvanderheijden? Or is there any case I could help bringing this PR forward?

theaungmyatmoe · 2024-04-18T04:06:48Z

I don't think the owner will make this PR forward, will you guys make this own community repo?

moufmouf · 2024-04-18T14:48:51Z

@aungmyatmoethegreat I will as a last resort. Before taking such steps, we should ask the repo owner if he needs some help. Maybe he saw the PRs but has no time to deal with those. I opened an issue to ask here: #21

ronvanderheijden · 2024-04-19T08:05:23Z

src/Laravel/PassportServiceProvider.php

@@ -35,26 +35,25 @@ public function boot()
            __DIR__ . '/config/openid.php' => $this->app->configPath('openid.php'),
        ], ['openid', 'openid-config']);

+        $this->loadRoutesFrom(__DIR__.'/routes/web.php');


I think we should only add the routes when the setting is enable in the config.

Good idea. I added in the config a routes entry and one can now select which routes should be enabled (I put the conditions in the web.php file)

src/Laravel/JwksController.php

ronvanderheijden · 2024-04-19T08:20:07Z

src/Laravel/routes/web.php

+Route::get('/oauth/jwks', \OpenIDConnect\Laravel\JwksController::class."@jwks")->name('openid.jwks');
+Route::get('/.well-known/openid-configuration', \OpenIDConnect\Laravel\DiscoveryController::class."@discovery")->name('openid.discovery');


Suggested change

Route::get('/oauth/jwks', \OpenIDConnect\Laravel\JwksController::class."@jwks")->name('openid.jwks');

Route::get('/.well-known/openid-configuration', \OpenIDConnect\Laravel\DiscoveryController::class."@discovery")->name('openid.discovery');

use OpenIDConnect\Laravel\JwksController;

use OpenIDConnect\Laravel\DiscoveryController;

Route::get('/oauth/jwks', [JwksController::class, 'jwks'])->name('openid.jwks');

Route::get('/.well-known/openid-configuration', [DiscoveryController::class, 'discovery'])->name('openid.discovery');

But an __invoke() is perhaps a better improvement.

Let's go for the __invoke!

theaungmyatmoe · 2024-04-23T17:35:39Z

src/Laravel/DiscoveryController.php

+
+class DiscoveryController
+{
+    public function discovery(Request $request)


Can't we use an invocable controller while calling a single method?

Suggested change

public function discovery(Request $request)

public function __invoke(Request $request)

theaungmyatmoe · 2024-04-23T17:39:45Z

src/IdTokenResponse.php

@@ -21,15 +21,18 @@ class IdTokenResponse extends BearerTokenResponse
    protected ClaimExtractor $claimExtractor;

    private Configuration $config;
+    private ?string $issuer;

    public function __construct(


I think we can use constructor property promotion in PHP 8.0. So that we can combine class fields, constructor definition and variable assignments all into one syntax, in the construct parameter list.

The package is still compatible with PHP 7.4 (that's what is declared in composer.json) so I guess this is a no go for this version at least.

theaungmyatmoe · 2024-04-23T17:44:06Z

src/Laravel/JwksController.php

+    }
+
+    private function getPublicKey(): string {
+        $publicKey = str_replace('\\n', "\n", app()->make(Config::class)->get('passport.public_key') ?? '');


In Larvel 11, config/passport.php is not published by default, so that we might use passport keypart to retieve the public key.

Suggested change

$publicKey = str_replace('\\n', "\n", app()->make(Config::class)->get('passport.public_key') ?? '');

$publicKeyFile = File::get(Passport::keyPath('oauth-public.key'));

$publicKey = str_replace('\\n', "\n", $publicKeyFile);

The public key file is not necessarily a file. It can come from env vars for instance. And I believe the config method will still read from the config file of teh package (even if the config is not published)

When using Laravel with Passport, we have enough knowledge of the environment to be able to provide 2 useful routes our of the box: - a discovery endpoint at `/.well-known/openid-configuration`. - a JWKS endpoint at `/oauth/jwks`. This greatly eases integrating with Laravel since clients can now use auto-discovery to integrate with Laravel + PassPort + OpenID.

Co-authored-by: Ron van der Heijden <[email protected]>

Routes can now be disabled from the config. Also, switching to using invokable routes.

moufmouf · 2024-04-24T09:16:17Z

@ronvanderheijden Thanks a lot for the review. I took your comments (and those of @aungmyatmoethegreat) in consideration and pushed the change. Tell me if it looks good. I'll rebase the other PRS depending on this one to apply the changes to all depending PRs.

theaungmyatmoe

LGTM

moufmouf · 2024-04-25T08:20:28Z

Ok, let's merge this! 👍

ronvanderheijden · 2024-04-25T18:59:55Z

This pull-request breaks the actions. I have fixed them in pull-requests #23 :)

moufmouf · 2024-04-29T07:20:42Z

Hey @ronvanderheijden,

Really sorry about that. I did not notice the actions. For some reason, they did not show up in the pull request on my side. I'll double check for the other PRs!

moufmouf changed the title ~~Adding well-known configuration and JWT routes in Laravel~~ Adding well-known configuration and JWKS routes in Laravel Jan 18, 2024

moufmouf force-pushed the autodiscovery branch from 5bedbe9 to 3be9e08 Compare January 18, 2024 17:23

This was referenced Jan 19, 2024

Adding support for a nonce #17

Merged

Discovery, JWKs and nounce support jeremy379/laravel-openid-connect#9

Closed

ronvanderheijden reviewed Apr 19, 2024

View reviewed changes

src/Laravel/JwksController.php Outdated Show resolved Hide resolved

ronvanderheijden reviewed Apr 19, 2024

View reviewed changes

theaungmyatmoe reviewed Apr 23, 2024

View reviewed changes

moufmouf and others added 3 commits April 24, 2024 11:14

Using the "config" utils instead of making the Config class.

6e57bf3

Co-authored-by: Ron van der Heijden <[email protected]>

Adding a parameter to make routes configurable.

7ca1517

Routes can now be disabled from the config. Also, switching to using invokable routes.

moufmouf force-pushed the autodiscovery branch from 7f7dc3b to 7ca1517 Compare April 24, 2024 09:14

GewoonYorick mentioned this pull request Apr 24, 2024

Adding well-known configuration and JWKS routes in Laravel jeremy379/laravel-openid-connect#12

Merged

theaungmyatmoe approved these changes Apr 25, 2024

View reviewed changes

moufmouf merged commit 9f56cce into ronvanderheijden:main Apr 25, 2024

moufmouf deleted the autodiscovery branch April 25, 2024 08:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding well-known configuration and JWKS routes in Laravel #16

Adding well-known configuration and JWKS routes in Laravel #16

moufmouf commented Jan 18, 2024

mrmstn commented Feb 5, 2024

theaungmyatmoe commented Apr 18, 2024

moufmouf commented Apr 18, 2024

ronvanderheijden Apr 19, 2024

moufmouf Apr 24, 2024

ronvanderheijden Apr 19, 2024

ronvanderheijden Apr 19, 2024

moufmouf Apr 24, 2024

theaungmyatmoe Apr 23, 2024

moufmouf Apr 24, 2024

theaungmyatmoe Apr 23, 2024

moufmouf Apr 24, 2024

theaungmyatmoe Apr 23, 2024

moufmouf Apr 24, 2024 •

edited

Loading

moufmouf commented Apr 24, 2024

theaungmyatmoe left a comment

moufmouf commented Apr 25, 2024

ronvanderheijden commented Apr 25, 2024

moufmouf commented Apr 29, 2024

		Route::get('/oauth/jwks', \OpenIDConnect\Laravel\JwksController::class."@jwks")->name('openid.jwks');
		Route::get('/.well-known/openid-configuration', \OpenIDConnect\Laravel\DiscoveryController::class."@discovery")->name('openid.discovery');

-Route::get('/oauth/jwks', \OpenIDConnect\Laravel\JwksController::class."@jwks")->name('openid.jwks');
-Route::get('/.well-known/openid-configuration', \OpenIDConnect\Laravel\DiscoveryController::class."@discovery")->name('openid.discovery');
+use OpenIDConnect\Laravel\JwksController;
+use OpenIDConnect\Laravel\DiscoveryController;
+Route::get('/oauth/jwks', [JwksController::class, 'jwks'])->name('openid.jwks');
+Route::get('/.well-known/openid-configuration', [DiscoveryController::class, 'discovery'])->name('openid.discovery');

	public function discovery(Request $request)
	public function __invoke(Request $request)

	$publicKey = str_replace('\\n', "\n", app()->make(Config::class)->get('passport.public_key') ?? '');
	$publicKeyFile = File::get(Passport::keyPath('oauth-public.key'));
	$publicKey = str_replace('\\n', "\n", $publicKeyFile);

Adding well-known configuration and JWKS routes in Laravel #16

Adding well-known configuration and JWKS routes in Laravel #16

Conversation

moufmouf commented Jan 18, 2024

mrmstn commented Feb 5, 2024

theaungmyatmoe commented Apr 18, 2024

moufmouf commented Apr 18, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moufmouf Apr 24, 2024 • edited Loading

Choose a reason for hiding this comment

moufmouf commented Apr 24, 2024

theaungmyatmoe left a comment

Choose a reason for hiding this comment

moufmouf commented Apr 25, 2024

ronvanderheijden commented Apr 25, 2024

moufmouf commented Apr 29, 2024

moufmouf Apr 24, 2024 •

edited

Loading