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

Powerschool: pull in email, user_type, and name data #22847

Merged
merged 1 commit into from Jun 5, 2018

Conversation

ewjordan
Copy link
Contributor

@ewjordan ewjordan commented Jun 1, 2018

Configure Powerschool SSO to pull in available data from the system. This comes in quite a bit differently from the way it does in other providers, so it takes a little more massaging to extract it.

Note that Powerschool does not appear to offer age or gender through this API, at least.


def extract_powerschool_data(auth)
# OpenID 2.0 data comes back in a different format compared to most of our other oauth data.
args = JSON.parse(auth.extra.response.message.to_json)['args']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Casting to JSON and parsing is not pretty, but I've had little luck interacting with the OpenID::Message object itself. This could probably use some cleanup eventually.

@ewjordan ewjordan merged commit bfd5f3e into staging Jun 5, 2018
Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

Any chance we should just be pulling in a gem that speaks OpenID 2.0 instead of doing this conversion ourselves?

# Fiddle with data if it's a Powerschool request (other OpenID 2.0 providers might need similar treatment if we add any)
if request.env["omniauth.auth"].provider.to_s == 'powerschool'
auth_hash = extract_powerschool_data(request.env["omniauth.auth"])
end
Copy link
Contributor

Choose a reason for hiding this comment

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

Style: It might be nice for auth_hash to become a private method on this controller, and the only place we reference request.env['omniauth.auth']. Then it'd be a fairly obvious place to add support for other OpenID 2.0 providers in the future.

email: args["[\"http://openid.net/srv/ax/1.0\", \"value.ext1\"]"],
name: {
first: args["[\"http://openid.net/srv/ax/1.0\", \"value.ext2\"]"],
last: args["[\"http://openid.net/srv/ax/1.0\", \"value.ext3\"]"],
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason not to use outer single-quotes here, so we don't have to escape the inner double-quotes?

skip_before_action :verify_authenticity_token, only: :powerschool

def extract_powerschool_data(auth)
# OpenID 2.0 data comes back in a different format compared to most of our other oauth data.
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be nice to add a link to documentation of the expected OpenID 2.0 data format here.

@ewjordan ewjordan deleted the powerschool-pull-vars branch June 7, 2018 04:09
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

Successfully merging this pull request may close these issues.

None yet

2 participants