redirect to admin login page when forward fails

[stefan0xC]

Currently, if the admin guard fails the user will get a 404 page instead of being forwarded to the login page.

This change addresses issues like #2880 and #2883 by implementing a default forward catcher for all admin pages so this comment is correct:

vaultwarden/src/api/admin.rs

Line 656 in f60a692

None => return Outcome::Forward(()), // If there is no cookie, redirect to login

[BlackDex]

I'm not sure if this will work as nicely.
Using this code, and when logged in, if i go to an invalid page lets say i go to /admin/userss/overview (note the extra s), that will also redirect me to the /admin page instead of showing me a 404.

Maybe the Request Guard should be change somehow, or we need to add extra functions which do not contain the AdminToken as a second rank which then redirects those back to the login page, which should in theory cause invalid endpoints to still show a 404 page instead of being redirected to the admin.

The "/<_..>" is to general in my opinion.
Unfortunately there is no way to have one function with multiple #[get()] attributes which would help instead of having to create specific functions for them all.

Now, I don't know if we are able to extract the registered routes, and match that within one single function, and based upon that either redirect to the admin login page, or return a 404. I haven't looked into it.

[BlackDex]

Also, something that just popped up into my mind.
It would even be nicer if, in the case someone wasn't logged-in, we can redirect that person back to the page they wanted to go to. So, if i go to /admin/diagnostics, but i have to login first, redirect that person to something like /admin?redirect=/admin/diagnostics and act upon that or something, that would even be more user friendly.

Maybe, we could just show the login page on the /admin/diagnostics page, and upon login use the referrer to redirect them to the correct page?

[BlackDex]

I haven't tested this my self yet.
So, no clue how this is working out, and if there are other ways on handling this.

But using a _ for a variable as a parameter is not that nice i think, only because Rust will complain if you are not using it, i think there is an other way, but not from the top of my head.

[stefan0xC]

While cleaning up my commit I noticed that the statement

vaultwarden/src/api/admin.rs

Lines 121 to 123 in 7a76731

	/// Used for `Location` response headers, which must specify an absolute URI
	/// (see https://tools.ietf.org/html/rfc2616#section-14.30).
	fn admin_url(referer: Referer) -> String {

is kinda outdated. I think in practice most browsers supported relative URIs but the RFC has been obsoleted since 2014 (see RFC7231) and since June 2022 there has been a new consolidating RFC9110, which both officiall allow for URI-references and thus also relative URIs in the Location header.

As explained by this rejected errata the use of relative URIs is intended and "accurately reflects real-world usage" so I think it's fine to go back to using admin_page().

[BlackDex]

While cleaning up my commit I noticed that the statement

vaultwarden/src/api/admin.rs

Lines 121 to 123 in 7a76731

	/// Used for `Location` response headers, which must specify an absolute URI
	/// (see https://tools.ietf.org/html/rfc2616#section-14.30).
	fn admin_url(referer: Referer) -> String {

is kinda outdated. I think in practice most browsers supported relative URIs but the RFC has been obsoleted since 2014 (see RFC7231) and since June 2022 there has been a new consolidating RFC9110, which both officiall allow for URI-references and thus also relative URIs in the Location header.

As explained by this rejected errata the use of relative URIs is intended and "accurately reflects real-world usage" so I think it's fine to go back to using admin_page().

MDN Also indicates the same: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Location

[stefan0xC]

Okay, I've squashed everything into a single commit because I think I'm done.

[BlackDex]

Hmm, this will break scripts which use the admin to modify users and orgs etc..
Like https://github.com/ViViDboarder/vaultwarden_ldap for example, but also custom user scripts who just POST a login to /admin.

I'm playing with it my self right now, and I think i have a nice solution. But unfortunately no time left today to continue working it out.

Basically, it kinda reverts almost everything, except the unauthorized catcher, and uses the render_admin_login with an extra Option<String> which contains the redirect value. But, for that to fully work, i also need to modify the post to of the form, and maybe some other stuff. But, that will have to wait for me at least.