ACS_DEFAULT_REDIRECT_URL has no effect

[pandafy]

It seems like LOGIN_REDIRECT_URL overrides ACS_DEFAULT_REDIRECT_URL setting.

On accessing saml2/login/ endpoint with no next parameter, the next_path defaults to LOGIN_REDIRECT_URL

djangosaml2/djangosaml2/views.py

Lines 135 to 137 in 02f4a19

	def get(self, request, *args, **kwargs):
	logger.debug('Login process started')
	next_path = self.get_next_path(request)

djangosaml2/djangosaml2/views.py

Lines 114 to 126 in 02f4a19

	def get_next_path(self, request: HttpRequest) -> str:
	''' Returns the path to put in the RelayState to redirect the user to after having logged in.
	If the user is already logged in (and if allowed), he will redirect to there immediately.
	'''
	next_path = settings.LOGIN_REDIRECT_URL
	if 'next' in request.GET:
	next_path = request.GET['next']
	elif 'RelayState' in request.GET:
	next_path = request.GET['RelayState']
	next_path = validate_referral_url(request, next_path)
	return next_path

This next_path is used to set value of relay_state here:

djangosaml2/djangosaml2/views.py

Lines 318 to 331 in 02f4a19

	if not http_response:
	# use the html provided by pysaml2 if no template was specified or it doesn't exist
	try:
	session_id, result = client.prepare_for_authenticate(
	entityid=selected_idp, relay_state=next_path,
	binding=binding, **sso_kwargs)
	except TypeError as e:
	_msg = f"Can't prepare the authentication for {selected_idp}"
	logger.error(f'{_msg}: {e}')
	return HttpResponse(_msg)
	else:
	http_response = HttpResponse(result['data'])
	else:
	raise UnsupportedBinding(f'Unsupported binding: {binding}')

Now in post method of AssertionConsumerServiceView, value of ACS_DEFAULT_REDIRECT_URL is retrieved, but that's get overridden by the RelayState parameter of POST request, which was set in saml2/login if I am not wrong.

djangosaml2/djangosaml2/views.py

Lines 500 to 511 in 02f4a19

	def build_relay_state(self) -> str:
	""" The relay state is a URL used to redirect the user to the view where they came from.
	"""
	login_redirect_url = get_custom_setting('LOGIN_REDIRECT_URL', '/')
	default_relay_state = get_custom_setting(
	'ACS_DEFAULT_REDIRECT_URL', login_redirect_url)
	relay_state = self.request.POST.get('RelayState', default_relay_state)
	relay_state = self.customize_relay_state(relay_state)
	if not relay_state:
	logger.warning('The RelayState parameter exists but is empty')
	relay_state = default_relay_state
	return relay_state

Hence ACS_DEFAULT_REDIRECT_URL has no result in this scenario.

[peppelinux]

I think that this issue and other two should be fixed in next release 1.3.0, I put it in this milestone.
Would you like to take this task and purpose a PR that solves this issue?

[pandafy]

Can you suggest how this should be solved? Should ACS_DEFAULT_REDIRECT_URL be given preference over RelayState parameter of POST request? I am bit clueless.

[peppelinux]

Sure, RelayState MUST have precedence and if it would be absent or None djangosaml2 would rely on ACS_DEFAULT_REDIRECT_URL

does it sound good to you?
Feel free to make your questions

[pandafy]

Sure, RelayState MUST have precedence and if it would be absent or None djangosaml2 would rely on ACS_DEFAULT_REDIRECT_URL

I believe this is what has been currently implemented. I guess the problem lies in the LoginView. Instead defaulting next_path to settings.REDIRECT_LOGIN_URL, it should also check ACS_DEFAULT_REDIRECT_URL .

[peppelinux]

what's your purpose on it?

we could make ACS_DEFAULT_REDIRECT first, before REDIRECT_LOGIN_URL and if it's None (undefined) it would rely on REDIRECT_LOGIN_URL but relaystate MUST have precedence on these.

tell me what would you prefer

[peppelinux]

@jaap3 did it here
https://github.com/IdentityPython/djangosaml2/pull/279/files#diff-14e9a5c86c429d4f469f5e22f7c2c4b239e52569a920f15c58513f2d2a216332R504

@pandafy does it looks good to you?
Feel free to close this issue or, if you can, to add an additional unit test to cover this
I've found a community here :)

[jaap3]

I think this needs some work before it can be resolved. Currently LOGIN_REDIRECT_URL seem to pretty much always win even though the djangosaml2 documentation claims the following:

The following setting lets you specify a URL for redirection after a successful
authentication:
ACS_DEFAULT_REDIRECT_URL = reverse_lazy('some_url_name')

It also claims the default is /, but it actually falls back to LOGIN_REDIRECT_URL, which in Django defaults to '/accounts/profile/' (which is a pretty stupid default, but it's been like that since forever) (https://docs.djangoproject.com/en/3.2/ref/settings/#login-redirect-url).

If @pandafy is not working on this I can probably find some time to continue from where I left of after #279 and add some helper function that gets the correct redirect url, taking both ACS_DEFAULT_REDIRECT_URL and LOGIN_REDIRECT_URL into account.

[jaap3]

Seeing as I only have a limited amount of time left to spend on this I decided it would be easier to just attempt to resolve this issue now :)

[pandafy]

Didn't get enough time yesterday to look into #279. From the changes I see in 3be1bbf, the issue seems to get fixed. Thank you very much @peppelinux and @jaap3.