[FIX] Auth/SOAP: Make SOAP authentication work again#11635
[FIX] Auth/SOAP: Make SOAP authentication work again#11635mjansenDatabay wants to merge 1 commit into
Conversation
thojou
left a comment
thojou
left a comment
There was a problem hiding this comment.
Hey @mjansenDatabay,
Thanks for the PR! Aside from the minor docblock type suggestions, I noticed a naming inconsistency: the component directory is components/ILIAS/SOAPAuth, but its namespace is declared as ILIAS\AuthSOAP.
Could we align these to follow a consistent convention? Let me know what you think.
Kind Regrads,
@thojou
| ]; | ||
| } | ||
|
|
||
| private function buildValidationResult(string $ext_uid, string $soap_pw, bool $new_user): array |
There was a problem hiding this comment.
I woule prefere a PHPStan return type annotation using an array shape like array{firstname: string, lastname: string, email: string, valid: bool} to this method for improved static analysis and explicit contract definition
| /** | ||
| * @param array<int|string, mixed> $arguments | ||
| * @return array<string, mixed> | ||
| */ | ||
| private function normalizeParameterKeys(array $arguments): array |
There was a problem hiding this comment.
Can we use a more strict array type definition instead of "mixed"?
| /** | ||
| * @param mixed[] $arguments | ||
| * @return array<string, mixed> | ||
| */ | ||
| private function invokeIsValidSession(array $arguments): array |
There was a problem hiding this comment.
Can we use a more strict array type definition instead of "mixed"?
| /** | ||
| * @param mixed[] $arguments | ||
| * @return array<string, mixed> | ||
| */ | ||
| public function __soapCall(string $function_name, array $arguments): array |
There was a problem hiding this comment.
Can we use a more strict array type definition instead of "mixed"?
|
|
||
| class SoapDummyAuthHandler | ||
| { | ||
| public function isValidSession(string $ext_uid, string $soap_pw, bool $new_user): array |
There was a problem hiding this comment.
Can we use a more strict array type definition instead of "mixed"?
2 participants
Triggered by the issue described in https://mantis.ilias.de/view.php?id=47821 , I had a closer look at the "SOAP Authentication" in ILIAS >= 10.x, which seemed broken because of fragile PHP code in the
nusoap_clientlibrary.SOAPAuthcomponent structure to align with ILIAS 10 conventions: public demo endpoints are now exposed viapublic/auth/soap/example/..., classes are insrc/, and namespaced underILIAS\AuthSOAP.SessionValidationClient,SessionValidationResult,SoapAuthEndpoint) to encapsulate endpoint building, SOAP request execution, and typed result handling.nusoap_clientto native PHPSoapClientwhile preserving wire compatibility (RPC/encoded mode, .NET-specific parameter names/soapaction handling).SoapServerhandles POST SOAP calls; nusoap remains for WSDL/non-POST handling to keep compatibility.soap_auth_use_dotnet(instead of mixed/incorrect key usage).If approved, this has to be picked/ported to
release_11andtrunk.