rootinc / laravel-azure-middleware

94 stars 39 forks source link

Having named route azure.callback fails callback now #56

Open RmK9 opened 2 years ago

RmK9 commented 2 years ago

Related to PR #53

First of all big thanks for @jimmypuckett for working on that PR, I was just looking at having this setup and did not even think about having multiple apps in Azure, we still just have 1 and with multiple redirect URLs, so this is an amazing inclusion.

For some reason though the callback fails after adding "->name('azure.callback')" - which is directly activating the "redirect_uri" addition (as per Jimmy not to break any older implementations).

Below is the error that is being received:

Client error: POST https://login.microsoftonline.com/f4bdacc7-1689-41e9-a95f-c131ceb881b9/oauth2/token resulted in a 400 Bad Request response:{"error":"invalid_grant","error_description":"AADSTS700009: Reply address must be provided when presenting an authorizat (truncated...)

And here is the final request URL:

https://login.microsoftonline.com/aaaaaaaa-1689-41e9-a95f-c131ceb881b9/oauth2/v2.0/authorize?response_type=code&client_id=aaaaaaaa-7878-aaaa-899c-8b394971d964&domain_hint=domain.com&scope=User.Read&redirect_uri=https%3A%2F%2Flocalhost%2Flogin%2Fazurecallback (modified IDs and domain_hint for privacy reasons)

Decoded URI is https://localhost/login/azurecallback which is an exact match to the one in Azure Portal:

azure portal urls

Is there something that I am doing wrong here? Pulled 'dev-master' version with composer and even used original routes as per Readme (instead of custom AppAzure middleware) and still the same.

Any help would be greatly appreciated, thank you!

jimmypuckett commented 2 years ago

@RmK9 I am sorry that you are having issues. I have a major demo in the morning for a client & we are polishing off a few things. I will try to read this over tonight, but maybe tomorrow. @jaredrhodes I am running the code in a dev branch, but there was a tweak that was requested in the review. I need to make 100% sure that I did not do something lame & not make that change in my dev branch of my project.

RmK9 commented 2 years ago

No worries at all, thanks for checking on this and good luck with the demo!

jaredrhodes commented 2 years ago

@RmK9 sorry to hear that, I put it on the dev-master branch so it could be more easily tested without making a "real" release. Things are going to be moving slowly on this repo, from my end, for a while. @jimmypuckett really appreciate the help! There's a chance I missed something that was requested and merged #53 too early. If you sort it out, note it or link a PR here.

RmK9 commented 2 years ago

Hey guys, been a while. I'm coming back to this after having a lot of side experience with Azure oath2 workflow and processes. Can tell what went wrong with the #53 PR. azurecallback method is missing 'redirect_uri' in it's 'form_params' array for the response. It must be an absolute decoded URL. So in my case it was just 'redirect_uri' => route('azure.callback') in the 'form_params' where route('azure.callback') is my named route Route::get('/login/azurecallback', '\App\Http\Middleware\AppAzure@azurecallback')->name('azure.callback');

Maybe @jimmypuckett had an override of the azurecallback method in his own extended middleware and forgot about this little detail, not quite sure.

You will also want to add a conditional addition of this 'redirect_uri' in the azurecallback method in case the named route does not exist, same as in the other methods.

gingebaker commented 1 year ago

Hello. I also stumbled on this issue. I also need multiple redirect uris from one app. As @RmK9 mentioned the code from current master just misses the fix in azurecallback function.

So for me changing 'form_params' in the azurecallback function (similar to the handle function):

$formParams = [
    'grant_type' => 'authorization_code',
    'client_id' => config('azure.client.id'),
    'client_secret' => config('azure.client.secret'),
    'code' => $code,
    'resource' => config('azure.resource'),
];
if(Route::has('azure.callback')) {
    $formParams['redirect_uri'] = route('azure.callback');
}

$response = $client->request('POST', $this->baseUrl . config('azure.tenant_id') . $this->route . "token", [
    'form_params' => $formParams
]);

does the job. Would be great if there would be a new release if this was fixed! Thank you.