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

amp-access onApplyAuthorizations callbacks. #16471

Merged

Conversation

gmajoulet
Copy link
Contributor

The STAMP implementation of amp-access prevents the user from navigating to a protected page (amp-access=<condition>).
It waits for the user to log in, and the document to be reauthorized before actually displaying the next page.

This PR implements a way for amp-story to know when the document has been reauthorized, so it can maybe navigate to the next page, if unblocked.

#12180

@newmuis
Copy link
Contributor

newmuis commented Jun 29, 2018

I defer to @dvoytenko, but this looks pretty straightforward and LGTM

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@942f226). Click here to learn what that means.
The diff coverage is 80%.

@@            Coverage Diff            @@
##             master   #16471   +/-   ##
=========================================
  Coverage          ?   78.11%           
=========================================
  Files             ?      549           
  Lines             ?    40038           
  Branches          ?        0           
=========================================
  Hits              ?    31274           
  Misses            ?     8764           
  Partials          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 942f226...07b4128. Read the comment docs.

Copy link
Contributor

@newmuis newmuis left a comment

Choose a reason for hiding this comment

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

Ping @dvoytenko

@newmuis
Copy link
Contributor

newmuis commented Aug 7, 2018

Let's go ahead with this PR. @gmajoulet can you rebase?

@gmajoulet gmajoulet force-pushed the access_authorizations_callback branch from 07b4128 to 37756b9 Compare August 7, 2018 17:51
@gmajoulet
Copy link
Contributor Author

Done :)

Side note: this is meant for the internal implementation of amp-story-access and can be changed later without breaking any publisher's implementation.

@newmuis newmuis merged commit 670b5c1 into ampproject:master Aug 7, 2018
kevinkassimo pushed a commit to kevinkassimo/amphtml that referenced this pull request Aug 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants