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

[fix #6488] Add FxA OAuth relier. #6489

Merged
merged 2 commits into from Nov 20, 2018

Conversation

jpetto
Copy link
Contributor

@jpetto jpetto commented Nov 19, 2018

Description

Adds FxA OAuth relier functionality. Also adds very basic templates for testing the flow. (All templates and corresponding URLs are behind a switch to test flow on stage, but wait until templates are finished for prod.)

Testing

https://www-demo5.allizom.org/firefox/concerts/

The above OAuth flow should work (redirecting you to /firefox/concerts/ with a cookie that will display a "Verified" message instead of the initial form/link) for all browsers, both signed in and out of FxA.

@jpetto
Copy link
Contributor Author

jpetto commented Nov 19, 2018

@jgmize and/or @metadave - As pmac is out this week, can one of you handle review on this? Don't worry much about the HTML or CSS - that's all placeholder until Craig finishes the front end.

Hoping to push this out to test on stage tomorrow (11/20).

Thanks!

Copy link
Contributor

@jgmize jgmize left a comment

Choose a reason for hiding this comment

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

Other than a couple of potential improvements we can add in future PRs, this LGTM. Nice work @jpetto!


var d = new Date();
d.setTime(d.getTime() + (cookieDays * 24 * 60 * 60 * 1000));
Mozilla.Cookies.setItem(stateStorageKey, state, d.toUTCString(), '/');
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably be using a signed cookie here, but it's not a blocker for this use case.

// add required params to the token fetch request
destURL += '?utm_campaign=firefox-concert-series-q4-2018&utm_source=mozilla.org';

fetch(destURL).then(function(resp) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally we'd use a circuit breaker here to handle a slow or unresponsive metrics endpoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As the metrics data here is not critical, I've removed the fetch call being a blocker. The user can now submit the form whenever they are ready. If the fetch call completed beforehand, they'll have metrics data.

@jgmize jgmize merged commit 46d127e into mozilla:master Nov 20, 2018
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