Add config for identity_providers in local_info.xml #524

Sae126V · 2024-09-11T07:17:03Z

config/local_info.xsd

gregcorbett · 2024-09-13T08:57:28Z

lib/Authentication/AuthTokens/ShibAuthToken.php

+                }
+
+                $this->principal = $_SERVER['voPersonID'];
+                $this->userDetails = ['AuthenticationRealm' => [$provider['idp']]];


This probably shouldn't just be the IdP. For EGI Check In, it should be "EGI Proxy IdP" - this should defined in the local config in the provider block.

Yep Make sense. Added this in 7e7396c

gregcorbett · 2024-11-14T10:48:05Z

config/local_info.xml

+        <authentication_realms>
+          <shib_realm_name>EGI Proxy IdP</shib_realm_name>
+        </authentication_realms>


This can just be authentication_realm

Suggested change

<authentication_realms>

<shib_realm_name>EGI Proxy IdP</shib_realm_name>

</authentication_realms>

<authentication_realm>

EGI Proxy IdP

</authentication_realm>

Updated in 01f3cd3

gregcorbett · 2024-11-14T10:49:21Z

config/local_info.xml

+        <authentication_realms>
+          <shib_realm_name>EGI Proxy IdP</shib_realm_name>
+        </authentication_realms>


As with https://github.com/GOCDB/gocdb/pull/524/files#r1842006878

Suggested change

<authentication_realms>

<shib_realm_name>EGI Proxy IdP</shib_realm_name>

</authentication_realms>

<authentication_realm>

EGI Proxy IdP

</authentication_realm>

Addressed as part of 01f3cd3

gregcorbett · 2024-11-14T10:52:16Z

lib/Authentication/AuthTokens/ShibAuthToken.php

+                if (empty($_SERVER['voPersonID'])) {
+                    die(
+                        "Did not receive required attributes from the "
+                        . "IDP $name to complete authentication. "


I think this will read better

Suggested change

. "IDP $name to complete authentication. "

. "$name to complete authentication. "

Done. 01f3cd3

gregcorbett · 2024-11-14T10:52:36Z

lib/Authentication/AuthTokens/ShibAuthToken.php

+                if (empty($_SERVER['entitlement'])) {
+                    die(
+                        "Did not receive the required entitlement "
+                        . "attribute from the IDP $name. "


I think this will read better

Suggested change

. "attribute from the IDP $name. "

. "attribute from the $name. "

Done 01f3cd3

gregcorbett · 2024-11-14T10:53:08Z

lib/Authentication/AuthTokens/ShibAuthToken.php

+                    ) {
+                        $HTML = "<ul>"
+                            . "<li>Login requires a GOCDB entitlement value "
+                            . "which was not provided for the IDP $name.</li>"


I think this will read better

Suggested change

. "which was not provided for the IDP $name.</li>"

. "which was not provided for the $name.</li>"

Done 01f3cd3

gregcorbett · 2024-11-14T10:54:10Z

lib/Authentication/AuthTokens/ShibAuthToken.php

+
+                $this->principal = $_SERVER['voPersonID'];
+                $this->userDetails = [
+                    'AuthenticationRealm' => $provider['authenticationRealms']


Given https://github.com/GOCDB/gocdb/pull/524/files#r1842006878, this will need a slight tweak

Done in 01f3cd3

gregcorbett · 2024-11-14T10:56:49Z

lib/Gocdb_Services/Config.php

+        $localInfo = $this->GetLocalInfoXML();
+        $identityProviders = [];
+
+        if (!empty($localInfo->identity_providers->provider)) {


What happens here if identity_providers is omitted in it's entirety from the local_config.xml?

identity_providers -> this is anyway optional. But if passed thru xml then it MUST have provider(This being idp, name, and other attributes) info.

This is just a safe check. Just in case if Null case is referring to any method(s) due to which it may cause errors.

gregcorbett · 2024-11-14T10:59:29Z

lib/Gocdb_Services/Config.php

+        if (!empty($localInfo->identity_providers->provider)) {
+            foreach (
+                $localInfo
+                    ->identity_providers
+                    ->provider as $providerDetails
+            ) {


Something like this I think would read better

Suggested change

if (!empty($localInfo->identity_providers->provider)) {

foreach (

$localInfo

->identity_providers

->provider as $providerDetails

) {

$configured_providers = $localInfo->identity_providers->provider;

if (!empty($configured_providers)) {

foreach ($configured_providers as $providerDetails) {

Done in 5091398

gregcorbett · 2024-11-14T10:59:55Z

lib/Gocdb_Services/Config.php

+
+                /** authentication_realms */
+                $authenticationRealms = [];
+                if ($providerDetails->authentication_realms) {


Given https://github.com/GOCDB/gocdb/pull/524/files#r1842006878, this will need a slight tweak

DOne. 5091398

gregcorbett · 2024-11-14T11:00:26Z

lib/Gocdb_Services/Config.php

+                $identityProviders[] = [
+                    'idp' => $idp,
+                    'name' => $name,
+                    'authenticationRealms' => $authenticationRealms,


Given https://github.com/GOCDB/gocdb/pull/524/files#r1842006878

Suggested change

'authenticationRealms' => $authenticationRealms,

'authenticationRealm' => $authenticationRealm,

Done 5091398

gregcorbett

A few minor changes are needed.

…Array

…/github.com/Sae126V/gocdb into GT-471-Shib/CheckIn-Token-reads-config-file

Add config for identity_providers in local_info.xml

e91226c

Sae126V commented Sep 13, 2024

View reviewed changes

config/local_info.xsd Outdated Show resolved Hide resolved

gregcorbett reviewed Sep 13, 2024

View reviewed changes

Sae126V force-pushed the GT-471-Shib/CheckIn-Token-reads-config-file branch 3 times, most recently from 694c0b2 to 7e7396c Compare September 13, 2024 10:48

Sae126V marked this pull request as ready for review September 13, 2024 10:53

Sae126V requested a review from a team as a code owner September 13, 2024 10:53

Update code to improve local_info schema

11b2a67

Sae126V force-pushed the GT-471-Shib/CheckIn-Token-reads-config-file branch from 7e7396c to 11b2a67 Compare September 16, 2024 08:10

gregcorbett reviewed Nov 14, 2024

View reviewed changes

gregcorbett requested changes Nov 14, 2024

View reviewed changes

gregcorbett self-assigned this Nov 14, 2024

gregcorbett added the enhancement label Nov 14, 2024

gregcorbett added this to the September 2024 milestone Nov 14, 2024

Sae126V and others added 4 commits December 4, 2024 13:40

Update code to use singular than plural for keys used in userDetails …

01f3cd3

…Array

Merge branch 'dev' into GT-471-Shib/CheckIn-Token-reads-config-file

5091398

Fix code climate issues

2a8cccf

Merge branch 'GT-471-Shib/CheckIn-Token-reads-config-file' of https:/…

965b3fa

…/github.com/Sae126V/gocdb into GT-471-Shib/CheckIn-Token-reads-config-file

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add config for identity_providers in local_info.xml #524

Add config for identity_providers in local_info.xml #524

Sae126V commented Sep 11, 2024 •

edited by jira bot

Loading

gregcorbett Sep 13, 2024

Sae126V Sep 13, 2024

gregcorbett Nov 14, 2024 •

edited

Loading

Sae126V Dec 4, 2024

gregcorbett Nov 14, 2024

Sae126V Dec 4, 2024

gregcorbett Nov 14, 2024

Sae126V Dec 4, 2024

gregcorbett Nov 14, 2024

Sae126V Dec 4, 2024

gregcorbett Nov 14, 2024

Sae126V Dec 4, 2024

gregcorbett Nov 14, 2024

Sae126V Dec 4, 2024

gregcorbett Nov 14, 2024

Sae126V Dec 4, 2024

gregcorbett Nov 14, 2024

Sae126V Dec 4, 2024

gregcorbett Nov 14, 2024

Sae126V Dec 4, 2024

gregcorbett Nov 14, 2024

Sae126V Dec 4, 2024

gregcorbett left a comment

	. "IDP $name to complete authentication. "
	. "$name to complete authentication. "

	. "attribute from the IDP $name. "
	. "attribute from the $name. "

	. "which was not provided for the IDP $name.</li>"
	. "which was not provided for the $name.</li>"

	'authenticationRealms' => $authenticationRealms,
	'authenticationRealm' => $authenticationRealm,

Add config for identity_providers in local_info.xml #524

Are you sure you want to change the base?

Add config for identity_providers in local_info.xml #524

Conversation

Sae126V commented Sep 11, 2024 • edited by jira bot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregcorbett Nov 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gregcorbett left a comment

Choose a reason for hiding this comment

Sae126V commented Sep 11, 2024 •

edited by jira bot

Loading

gregcorbett Nov 14, 2024 •

edited

Loading