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

Generalize the plugin to make it easier to add new providers #18

Merged
merged 14 commits into from
Aug 21, 2017

Conversation

shankari
Copy link
Contributor

We need this for two use cases:
In particular, we want to support:

  • OpenID Connect, for @Andrew-Tan
  • "dev mode" with token = user email, for testing against localhost

And from the `AuthCompletionHandler` interface.
Instead, wrap all references into the `AuthCompletionHandler` implementation

Testing done:
- initial signin + refresh both work
And from the interface to `GoogleAccountManagerAuth`.
All references are only in the implementation.
@shankari
Copy link
Contributor Author

Related communication changes
e-mission/cordova-server-communication#17

And switch the existing google sign-in auth completion handler to implement
that protocol. All external references now use the factory directly.
Almost there!
And switch the existing google sign-in auth completion handler to implement
that protocol. All external references now use the factory directly.
Almost there!

These are the android changes
Files represent the creator and creator factory interfaces from
847905d and
268d52e
shankari added a commit to shankari/e-mission-data-collection that referenced this pull request Aug 20, 2017
From the specific google implementation used earlier
This corresponds to the changes in
e-mission/cordova-jwt-auth#18
The main difference is that instead of passing the activity (on android) or the
viewcontroller (on iOS), we pass the plugin as an argument to the token
creator, *for the signIn case*.

For anything other than the sign in case, we don't actually need any UI stuff,
so this allows us to avoid the awkward instanceof check for activity on
android, and the equally awkward need to remember to set the view controller on iOS.

And passing in the plugin also allows us to set activity result callback from
the google-specific implementation instead of the generic plugin (we may not
use the activity callbacks for all implementations) and makes it easier to
implement the dev implementation that needs to execute javascript.

Includes minor fix to the way that files are copied over.
…implementation

`AuthCompletionHandler` is a really generic name for something that is really
the google-specific implementation. Rename to match the android code, specially
when we're reinstalling the plugin anyway
We renamed `AuthCompletionHandler` to `GoogleSigninAuth` in
f9b82d8. This fixes the code to
return use it properly.

Checking in as separate commit to capture the `git mv` properly
obvs I need the header, because otherwise the return type of `getInstance` is
unknown. Was removed in a2f008a
…llback

In that case, `tokenCreator` is null and we shouldn't crash.
Instead, we can just ignore the call to the custom URL.
In this, the user simply enters their email address. If the email address
matches one that was used to load data before this, the account is re-used,
which makes it easier to review and re-test data.

It also ensure that sessions are maintained across app restarts without logging
in every time, which makes debugging a lot easier.

And finally, it is an example of a javascript-based auth callback, which should
make it much easier to integrate with other auth systems if they don't already
have nice libraries to do it for us.

NOTE: the javascript stuff is extremely kludgy and ugly-looking, and I will
clean it up shortly after some more experimentation.
This should have been part of
4d5eecc
but I forgot :(
- Unifies the code between ios and android
- Makes it easier to see (wrapping everything in " for the code strings and
  then ' for the strings in the code was getting confusing)
- This also makes it easier to see multi-line code
- We can also attach the debugger to the function, which might be useful as
  this gets more complex
@shankari shankari merged commit d04a504 into e-mission:master Aug 21, 2017
@shankari shankari deleted the switch_to_new_auth branch August 22, 2017 00:00
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.

1 participant