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

Improve logging #88

Closed
SteelWagstaff opened this issue Sep 24, 2021 · 3 comments
Closed

Improve logging #88

SteelWagstaff opened this issue Sep 24, 2021 · 3 comments
Assignees

Comments

@SteelWagstaff
Copy link
Member

SteelWagstaff commented Sep 24, 2021

We have a few problems with the logs:

  1. We're logging information a little too late:
    $this->logData( 'email from SAML attributes', [ $email ] );
    $this->logData( 'net_id from SAML attributes', [ $net_id ] );
    $this->logData( 'SAML Settings', [ $this->getSettingsWithoutCertificatesAndPrivateKey() ] );

    It would be good to log the contents of self::USER_DATA at the same time we store it to the $attributes variable so we can see what values we have from the claim even before we attempt to set the username and email address. We should have information in the log about the claim information received by Pressbooks in cases when login fails (maybe especially so in this cases). We should also switch the order of lines 369 and 368 so that we log net_id before we log email.
  2. We're unneccesarily duplicating some logging information
    Screenshot from 2021-09-23 22-00-06
    Each successful login sends the same information twice, except once with some "Auth SAML data" and once without it. See
    $this->logData( 'Auth SAML data', $log_auth_data, true );
  3. The post authentication log already includes all of the information in the two earlier log statements. Not sure why we're producing and recording all three of them.

It would be good to allow network managers to download and inspect these logs from the SSO config page, as we allow with the OIDC plugin's logs.

Finally, logs do not appear to be automatically configured the plugin is activated or used in our production network. Not sure what needs to be done so that necessary S3 buckets or CloudWatch groups are created, but if the plugin is active on one of our hosted networks, logs should automatically be kept.

@SteelWagstaff
Copy link
Member Author

Things we attempt to log:

  1. email, net_id (uid), SAML settings after authentication (if net_id exists):
    $this->logData( 'email from SAML attributes', [ $email ] );
    $this->logData( 'net_id from SAML attributes', [ $net_id ] );
    $this->logData( 'SAML Settings', [ $this->getSettingsWithoutCertificatesAndPrivateKey() ] );
  2. Errors from SAML Auth, Last SAML Error Reason, during authentication if we have errors:
    $this->logData( 'Errors from SAML Auth', $errors );
    $this->logData( 'Last SAML Error Reason', [ $message ], true );
  3. NameID of the assertion, NameID SP NameQualifier of the assertion (after authentication):
    $this->logData( 'NameID of the assertion', [ $this->auth->getNameId() ] );
    $this->logData( 'NameID SP NameQualifier of the assertion', [ $this->auth->getNameIdSPNameQualifier() ], true );
  4. Auth SAML data (just after we storeAuthDataInSession):
    $this->logData( 'Auth SAML data', $log_auth_data, true );
  5. Cookies, Username matched, Session after logged [Matched] after a matching user if found and logged in:
    $this->logData( 'Cookies', $this->getPartialCookies() );
    $this->logData( 'Username matched', [ $user->user_login ] );
    $this->logData( 'Session after logged [Matched]', [ $_SESSION ], true );
  6. User metadata stored (after a user has been linked to their SAML identity):
    $this->logData( 'User metadata stored', [ $user_id, $condition ] );
  7. Cookies, Username associated, Session after logged [Associated] after a new user if created and logged in:
    $this->logData( 'Cookies', $this->getPartialCookies() );
    $this->logData( 'Username associated', [ $username ] );
    $this->logData( 'Session after logged [Associated]', [ $_SESSION ], true );

Things included in first log statement generated for each successful login attempt:

  • NameID of the assertion
  • NameID SP NameQualifier of the assertion (3rd item in list above only)

Things included in second log statement (generated at almost the same time as first statement)

  • NameID of the assertion
  • NameID SP NameQualifier of the assertion
  • Auth SAML data (3rd and 4th items in list above)

Things included in third log statement:
if user matched:

  • email
  • net_id (uid)
  • SAML settings
  • Cookies
  • Username matched
  • Session after logged [Matched] (1st and 5th items in list above)

if new user created

  • email
  • net_id (uid)
  • SAML settings
  • User metadata stored
  • Cookies
  • Username associated
  • Session after logged [Associated] (1st, 6th and 7th items in list above)

@SteelWagstaff
Copy link
Member Author

Logs look great for SamlTest on integrations. Will try to test with alternate IdP

@SteelWagstaff
Copy link
Member Author

Client IdP I hoped to test with is down. Without ability to test further, will consider the issue resolved.

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

3 participants