Skip to content
This repository was archived by the owner on Oct 29, 2020. It is now read-only.

Log all the stars#6262

Merged
DFurnes merged 8 commits intoDoSomethingArchive:devfrom
DFurnes:log-all-the-stars
Mar 11, 2016
Merged

Log all the stars#6262
DFurnes merged 8 commits intoDoSomethingArchive:devfrom
DFurnes:log-all-the-stars

Conversation

@DFurnes
Copy link
Contributor

@DFurnes DFurnes commented Mar 11, 2016

What's this PR do?

Hello Drupal, my old friend. This PR adds request logging for Northstar update/verify requests, in the hopes that we can figure out why so many users can't be mock authenticated. 🔍

How should this be manually tested?

If you want to pull this branch down to your local and run the ol' drush updb -y, any attempts to login or update your profile should log the corresponding Northstar requests to the dosomething_northstar_request_log table. Like so:

screen shot 2016-03-11 at 10 50 11 am

Any background context you want to provide?

Nah. See the relevant ticket below.

What are the relevant tickets?

References #6260.


For review: @angaither

DFurnes added 5 commits March 11, 2016 10:29
This will log times when we update a user’s Northstar profile… such as
when they update their profile on Phoenix or reset their password.
// Add to request log
if(variable_get('dosomething_northstar_log')) {
// Don't log plaintext password.
$credentials['password'] = '*****';
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just unset this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we could do that. On the other hand, I wonder if it's worth doing an isset check and only masking the password like this if the field is in the payload... could catch if we're somehow not sending it on every request.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh interesting! I like

@DFurnes
Copy link
Contributor Author

DFurnes commented Mar 11, 2016

@angaither Made updates based on your comments. I also added logging to the "register" action.

@angaither
Copy link
Contributor

👍

DFurnes added a commit that referenced this pull request Mar 11, 2016
@DFurnes DFurnes merged commit 22974c4 into DoSomethingArchive:dev Mar 11, 2016
@DFurnes DFurnes deleted the log-all-the-stars branch March 11, 2016 20:09
@DFurnes DFurnes mentioned this pull request Mar 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants