statamic / cms

The core Laravel CMS Composer package
https://statamic.com
Other
4.12k stars 540 forks source link

OAuth redirect hardcoded to the dashboard #7654

Open jonassiewertsen opened 1 year ago

jonassiewertsen commented 1 year ago

Bug description

At the moment, all OAuth redirects are hardcoded to the Statamic Dashboard. So redirects are not working correctly.

https://github.com/statamic/cms/blob/3.4/resources/views/auth/login.blade.php#L16

It would be helpful if a custom url could be set, allowing to redirect.

This needs more testing and maybe a cleaner syntax, but something like this might already do it:

 <a href="{{ $provider->loginUrl() }}?redirect={{ parse_url(session()->get('some.good.session.naming') ?? cp_route('index'))['path'] }}" class="btn block btn-primary">

If no session has been set, we'll redirect to the dashboard as usual.

@jasonvarga Would you accept a PR in that direction? If so, I will happily create one and take a deeper look, how an option like this could be integrated smoothly.

How to reproduce

If using OAuth via socialite:

  1. Logout if not already
  2. Go to fx. /globals
  3. You will get redirected to the login
  4. The redirect parameter will be dashboard and can not be changed.

Logs

No response

Environment

Current Statamic Version: 3.4.5

Installation

Fresh statamic/statamic site via CLI

Antlers Parser

None

Additional details

No response

jasonvarga commented 1 year ago

Just to clarify what you mean:

When you try to visit a CP url while logged out, you get redirected to the login screen, and once you login you should be taken back to that original url.

It does that for the regular login form but not for the oauth buttons.

Then yes absolutely would accept a PR. Nice find.

The return url logic is already available for you in the $referer variable. I haven't tested it, but this should work.

-{{ $provider->loginUrl() }}?redirect={{ parse_url(cp_route('index'))['path'] }}
+{{ $provider->loginUrl() }}?redirect={{ $referer }}

That parse_url bit seems unnecessary - but maybe not. Can't quite remember.

jonassiewertsen commented 1 year ago

That's great feedback! I did miss the $referer indeed, which might already do the trick.

I will test that and see if we need the parse_url method at all.

I'll create a PR soon.