Redirect specification should be optional

Steven Jones → made their first commit to this issue’s fork.

I've added the intention of the patches here to the issue fork, and I'll open a MR in a moment.

The headlines are:

• Tame the crazy configuration form a little.
• Make it so that you do not have to specify a value for each and every role.
• When logging in, loop over the configured roles in reverse and see if the user has that role, in which case, do the redirect.
• Do not fallback to doing a redirect. It seems to me that if you've opted out of that functionality, we should respect it.

Created a merge request.Please check it

@uttam it would be good to explain the difference between my MR, and yours, and what the intention is here, are you offering a different solution to the original problem etc.?

I think removing the default attribute of the form is enough for the module to work as expectation.In the previous MR I tried to validate the path but later i realized that was not working as expectation so I created another MR where i just remove the default attribute

I think removing the default attribute of the form is enough for the module to work as expectation

I don't think so, because on login, the module only checks a single role to determine what redirect to do. So, if you'll likely end up needing to fill in all the values in the form anyway.

Rather than contributing a new approach, how about reviewing or improving the one already in the merge request !8?

Okay steven I will work on it and thanks for the explanation.

Hi @Steven Jones I agree with the suggestion on comment #17 to not use a fallback if authenticated role is also empty and consider it as opt out option.

I have rebased the MR merge request !8 , resolved the conflicts and a minor change in the code to set "destination" query parameter for redirection.

I have verified the MR patch from merge request !8 it works as expected , approach looks good to me. Keeping it in NR status for now for further review.

Applied this MR 8 , MR works fine . Redirect specification is optional and it works well for specified users . RTBC++.

james.williams → made their first commit to this issue’s fork.