Currently the module support defining custom login addresses and also provides an event that allows to customize further the redirection (by adding extra query parameters and so on).

Ideally it should be possible to do the same during the validation step of the process.

The proposed solution is to add a new parameter in the configuration that will store a custom validation path. This validation path will be used instead of the predefined ones ONLY if it is set, if it is empty it will fall back to the default ones.
In addition, a PreValidateEvent will be triggered. This trigger would allow the customize the $validate_url and $param parameters.

Comments

ieguskiza created an issue. See original summary.

bkosborne’s picture

Why do you need a custom validation path? If your CAS server implements the CAS protocol appropriately, then the paths are all the same. What CAS server software are you using?

ieguskiza’s picture

Hi there, the CAS server we use follows the protocol but also provides alternate options that allow more fine-grained authentication.
I agree it may sound too specific but at the very least I think there should be a PreValidateEvent, if only for consistency's sake since there already is a PreRedirectEvent as well.

ieguskiza’s picture

Status: Active » Needs review
StatusFileSize
new6.35 KB

I uploaded a patch to showcase the proposed solution.

Status: Needs review » Needs work

The last submitted patch, 4: 3001882-4.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

ieguskiza’s picture

StatusFileSize
new6.52 KB

Apologies, the previous patch was not the correct one.

bkosborne’s picture

I think the new pre validate event should handle all the customizations. We shouldn't add the ability to specify a unique validation URL in the admin config. I think the admin config should satisfy the needs of 90% of the CAS setups, and I don't think yours is a common one. The pre validate event can have a method to control the base URL and path.

ieguskiza’s picture

Status: Needs work » Needs review
StatusFileSize
new4.13 KB

Yeah that makes sense. I uploaded a patch where the event allows to modify both the path and the parameters. Let me know what you think.

Status: Needs review » Needs work

The last submitted patch, 8: 3001882-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

ieguskiza’s picture

Status: Needs work » Needs review
StatusFileSize
new7.6 KB

Fixed the test and added a separate one for the new event.

upchuk’s picture

StatusFileSize
new7.61 KB

Made some small corrections. Nothing major.

bkosborne’s picture

StatusFileSize
new1.24 KB
new0 bytes

Looks good. I just modified the comments.

bkosborne’s picture

StatusFileSize
new7.84 KB

Oops empty patch

The last submitted patch, 12: 3001882-12.patch, failed testing. View results

  • bkosborne committed c171e22 on 8.x-1.x
    Issue #3001882 by ieguskiza, bkosborne, Upchuk: Support custom server...
bkosborne’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.