fix: consider agent level redirect URL

[ctron]

Closes: #23

[kate-shine]

Works really well now, thank you <3

The only thing this sadly means for us when using a provider that doesn't do wildcards is the loss of redirect feature from authorized pages. I wonder how that could be solved. Maybe the original URL could be encoded in state that is sent to auth provider, which would allow us to retain it and redirect back to it at the redirect page? If it sounds like a useful idea and there would be an interest of having that, I could try to implement and PR it.

[ctron]

If it sounds like a useful idea and there would be an interest of having that, I could try to implement and PR it.

That definitely sounds like a good idea.

Another way to deal with this may be to store this in the current session:

yew-oauth2/src/agent/mod.rs

Lines 443 to 450 in 62187a9

	SessionStorage::set(STORAGE_KEY_CSRF_TOKEN, login_context.csrf_token)
	.map_err(|err| OAuth2Error::StartLogin(err.to_string()))?;
	SessionStorage::set(STORAGE_KEY_LOGIN_STATE, login_context.state)
	.map_err(|err| OAuth2Error::StartLogin(err.to_string()))?;
	SessionStorage::set(STORAGE_KEY_REDIRECT_URL, redirect_url)
	.map_err(|err| OAuth2Error::StartLogin(err.to_string()))?;

And then restore it later on:

yew-oauth2/src/agent/mod.rs

Lines 321 to 325 in 62187a9

	let redirect_url = Self::get_from_store(STORAGE_KEY_REDIRECT_URL)?;
	log::debug!("Redirect URL: {redirect_url}");
	let redirect_url = Url::parse(&redirect_url).map_err(|err| {
	OAuth2Error::LoginResult(format!("Failed to parse redirect URL: {err}"))
	})?;

…

And the more I look at this code, I am wondering what "redirect URL" should actually accomplish in this case. I've the weird feeling that this stored "redirect URL" actually should be the redirect URL that the request should redirect to after coming back to the redirect to URL from the SSO instance.

[ctron]

Ah no. That records the redirect URI which was actually used. In cases where the user might provide a custom one on the start_login_opts call.

So, it might make sense to have a "post redirect URL", which you fill with Self::current_url(). And when coming back, detecting that state, and the actual URL not being equal to the stored one, direct the browser the browser with the history API.

[ctron]

I released this as 0.9.3. If you have a PR, I can just do another release. Thanks for testing!

[kate-shine]

Thanks for release :) I will try to play with this further as soon as possible

[AlexandreRoba]

Hi @ctron, I tried the latest version (0.9.3) and indeed the return_url is now properly used if specified in the login options.

So, it might make sense to have a "post redirect URL", which you fill with Self::current_url(). And when coming back, detecting that state, and the actual URL not being equal to the stored one, direct the browser the browser with the history API.

I agree with your analysis. But not sure using the history would be the best solution. What about setting this posturl as query string parameter to the url which is specified in the login options? We could have a flag on login options that allow to enable this and maybe a parameter name? I checked couple of other IDPs and apparently the allowed returns url are often a fixed list but query string parameters are not used as discriminant.

I'm going to give it a second try to make a pull request but I'm a beginner with Rust and Yew :p

[kate-shine]

I don't exactly like the idea of polluting URL with it unnecessarily, and I really don't know how will IDP react to that, because it inputs query strings on its own (State and others). I will try your approach with MS Entra, but ultimately, the solution that doesn't rely on IDP to handle everything the way you need feels more reliable.

[kate-shine]

The redirect URI 'http://localhost:8080/?foo=meow' specified in the request does not match the redirect URIs configured for the application

[AlexandreRoba]

@kate-shine I'm not sure we are talking about the same thing. When making the authentication the login url includes a returnUrl with the path to the ressource the user should be redirected to after login. It is this value that would have add an extra query string. This value is UrlEncoded and it is treated as a String by IDP but validated agains "allowed" return value. So basically the login url would have looked like` https://idp.com?state=xxx&code=yyy&returnURL=....

But you are right about the fact that some IDP might also be stricter than other and validate returnURl with query string...

To make it safer. I made a pull request that store this original url in the session store... This way you only need to configure the allowed urls on your IDP to a single value. The returnURL will always be the same.

Hoppefyll @ctron can review this soon. The pull request is working for me.

[kate-shine]

I really forgot about that one, sorry :(