Added redirect_uri to form_parms in Azure.php. This fixes the 400 Bad…

[MartinDKnudsen]

When trying to provide an redirect url to prams, error comes back:

Client error: POST https://login.microsoftonline.com/47e303e7-e5fa-4519-a069-28a5a61c57d8/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...)`

This is because the form_parms need a redirect uri. Added that to make sure the error no longer occur.

[shealavington]

I'm in need of this exact change, went to send the PR myself.

[janzikmund]

Could we have a new release, please? The fixes for redirect_uri are not included in currently latest 0.9.10. So if the package is installed as per the readme using composer require rootinc/laravel-azure-middleware, they are missing. I was already about to do a PR myself before figured out it actually exists in master branch.

[janzikmund]

@cwp-sheal - I believe this PR is now obsolete and can be closed - it triggers an error if the route('azure.callback') does not exist.

Please see my PR here, which addresses this in a better way and only appends the parameter if the azure.callback route exists.

[shealavington]

@cwp-sheal - I believe this PR is now obsolete and can be closed - it triggers an error if the route('azure.callback') does not exist.

Please see my PR here, which addresses this in a better way and only appends the parameter if the azure.callback route exists.

The callback should always exist anyway, as Azure needs a return URL (which is azure.callback), it seems like you've just added redundant logic.

I stand by this PR.

[janzikmund]

@shealavington - personally I don't care, as I also use the azure.callback, but please see the reasons on this PR. That PR has been accepted, so I assume there is a reason why the azure.callback should stay optional and why current master branch already contains this code for the middleware validation.

I would be happy if either of our PRs got accepted, just believe mine is closer to existing implementation in master branch. Actually it's pretty much a copy of the functionality that is already there, just for a different call.

[shealavington]

@shealavington - personally I don't care, as I also use the azure.callback, but please see the reasons on this PR. That PR has been accepted, so I assume there is a reason why the azure.callback should stay optional and why current master branch already contains this code for the middleware validation.

I would be happy if either of our PRs got accepted, just believe mine is closer to existing implementation in master branch. Actually it's pretty much a copy of the functionality that is already there, just for a different call.

I appreciate the reference and yes, from that linked issue, fair enough, optional probably is better then. As you say, making this PR redundant.

An aggressive approach to a conversation, but fair points, 4/10 for friendliness, 10/10 for the PR.