Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GT-56: Add base class for OIDC AuthTokens, add EOSCAAI AuthToken #430

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from

Conversation

gregcorbett
Copy link
Member

This PR:

  • adds an abstract class that future OIDC based IDP integrations can extended.
  • provides an concrete class for integrating the EOSC (demo) IDP via OIDC

Rather than proliferate die() calls, the OIDC abstract class throws exceptions which are now handled (granted this handling then calls die() because it can't make use of show_view('error.php',...)

This code is running on gocdb-preprod.eosc-portal.eu.

Supersedes #348.

- it's abstract because the idea is that it will be extended by
  specific, non-abstract, classes - like a token class to handle
  EOSC AAI.
- currently only supports aai-demo.eosc-portal.eu
@gregcorbett
Copy link
Member Author

The codacy / codeclimate issues are being tackled as part of #423.

@gregcorbett gregcorbett changed the title Add base class for OIDC AuthTokens, add EOSCAAI AuthToken GT-56: Add base class for OIDC AuthTokens, add EOSCAAI AuthToken Feb 22, 2023
gregcorbett and others added 3 commits February 27, 2023 16:05
Co-authored-by: Rowan <rowanmoss04@gmail.com>
- it was over 80 characters, which PSR12 says we should not
  do.
- it also makes it more consistent with other lines in this file.

Co-authored-by: Rowan <rowanmoss04@gmail.com>
@gregcorbett gregcorbett requested a review from rowan04 April 4, 2023 15:25
/**
* {@see IAuthentication::eraseCredentials()}
*/
public function eraseCredentials()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this function and validate() function below left blank?

$_SERVER[$this->groupHeader]
);

$presentBadGroups = [];
Copy link
Member

@rowan04 rowan04 Apr 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this block of code start with a comment, like in the similar function checkRequiredGroups() above?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saying something like "// Build up a list of banned groups"

null,
'You do not belong to the correct group(s) ' .
'to gain access to this site.<br /><br />' . $errorContext .
$prependString . $groupString . '<br /><br />' . $this->helpString
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is $prependString needed here, when $groupString starts with $prependString?

@gregcorbett gregcorbett modified the milestones: May 2023, September 2023 Jan 30, 2024
$this->groupSplitChar = ',';
$this->bannedGroups = array();
$this->requiredGroups = array("urn:geant:eosc-portal.eu:res:gocdb.eosc-portal.eu");
$this->helpString = 'Please seek assistance by opening a ticket against the ' .
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$this->helpString = 'Please seek assistance by opening a ticket against the ' .
$this->helpMessage = 'Please seek assistance by opening a ticket against the ' .

helpMessage is probably more readable than helpString

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants