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

Feature Request: Add different Exception Types for different errors #250

Open
Sebi2020 opened this issue Jan 28, 2021 · 3 comments
Open

Comments

@Sebi2020
Copy link

Sebi2020 commented Jan 28, 2021

Currently it's really difficult to handle different OpenID errors because the library uses only one exception class. Additionally it does not use exception codes thus requireing the developer to do string matching to actually figure out which error caused the exception.

Please introduce exception codes or different exception classes for different exceptions.

@Sebi2020 Sebi2020 changed the title Feature Request: Add different Excception Types for different errors Feature Request: Add different Exception Types for different errors Jan 28, 2021
@huchim
Copy link

huchim commented May 6, 2021

  • Require the CURL and JSON PHP extensions to be installed:
    • OpenIDConnect needs the CURL PHP extension.
    • OpenIDConnect needs the JSON PHP extension.
  • Do a preemptive check to see if the provider has thrown an error from a previous redirect: Error: xxxxx Description: lorem ipsum
  • Throw an error if the server returns one:
    • $token_json->error_description)
    • 'Got response: ' . $token_json->error
  • Do an OpenID Connect session check: Unable to determine state
  • User did not authorize openid scope.: User did not authorize openid scope.
  • Verify the signature:
    • Unable to verify signature due to no jwks_uri being defined
    • Unable to verify signature
      • Error token is not a string
      • Error missing part 0 in token
      • Error decoding signature from token
      • Error decoding JSON from token header
      • Error decoding JSON from jwks_uri
      • Error missing signature type in token header
      • No support for signature type
    • Unable to verify JWT claims
  • If the configuration value is not available: The provider {$param} could not be fetched. Make sure your provider has a well known configuration available.
  • Random token generation failed: Random token generation failed.
  • get_key_for_header():
    • Unable to find a key for (algorithm, kid)
    • Unable to find a key for RSA
  • verifyRSAJWTsignature():
    • Crypt_RSA support unavailable.
    • Malformed key object
  • verifyHMACJWTsignature(): hash_hmac support unavailable.
  • requestUserInfo(): The communication to retrieve user data has failed with status code
  • fetchURL(): Curl error: (XXXX)
  • getIssuer(): The issuer has not been set
  • getProviderURL: The provider URL has not been set
  • register():
    • Error registering: JSON response received from the server was invalid.
    • $json_response->{'error_description'}
    • Error registering: Please contact the OpenID Connect provider and obtain a Client ID and Secret directly from them

I could suggest to consider "provider exceptions", "validation exceptions", "server exceptions", "network exception" and left "generics exceptions" as OpenIDConnectClientException

  • OpenIDConnectProviderException
  • OpenIDConnectValidationException
  • OpenIDConnectNetworkException
  • OpenIDConnectServerException

Maybe a static method:

private static function throws(Throwable $exception) {
    throw new OpenIDConnectClientException('Errror', 0, $exception);
}

private function foo() {
   self::throw(new OpenIDConnectProviderException('Unable to determine state');
}

@Sebi2020
Copy link
Author

Sebi2020 commented May 7, 2021

I want to add that meaninigful error codes should be provided as well.

@huchim
Copy link

huchim commented May 7, 2021

I'm coming from Code Triage and I'm not the repo owner, but I can creates a new PR and ask to owner if he is agree with change.

cc: @jumbojett

huchim added a commit to huchim/OpenID-Connect-PHP that referenced this issue May 7, 2021
This PR is just to suggest an error factory suggested in feature request jumbojett#250  It doesn't covers all exceptions because I want to know if this can help or not.  It creates an exception factory in order to throw specific exceptions and user can handle exceptions without to compare messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants