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

Custom scopes for different requirements (Android and iOS) #269

Merged
merged 9 commits into from
May 1, 2024

Conversation

AlvinTCH
Copy link
Contributor

@AlvinTCH AlvinTCH commented Apr 6, 2023

Having custom scopes whenever the function initializes

I have an app whereby there are different scope requirements for different functions of the app so as to not request excessive scopes from the user.

For example,

  1. the part whereby I just need the user to log in, the scope of ['email', 'profile'] is sufficient.
  2. the part whereby I need access to the user's calendar, I need the scope of ['email', 'profile', 'https://www.googleapis.com/auth/calendar']

There is currently no convenient way to do this in the app as it takes the scope from the capacitor config (i.e capacitor.config.json)

So in this implementation, I am taking the data from the GoogleAuth.initialize function for reusability

In the component that requires login, initialize GoogleAuth with the appropriate parameters they need

import { GoogleAuth } from '@codetrix-studio/capacitor-google-auth';

// use hook after platform dom ready
GoogleAuth.initialize({
  clientId: 'CLIENT_ID.apps.googleusercontent.com',
  scopes: ['profile', 'email'],
  grantOfflineAccess: true,
});

The general idea for this implementation:

  1. Move the load function to a function on its own (loadSignInClient)
  2. Under the initialize function, get the custom params from the call (GoogleAuth.initialize), with the data from capacitor.config.json as the fallback
  3. Pass the applicable parameters to loadSignInClient and use these applicable parameters to initialize the Google Signin Client

Testing

Currently, I have tested this implementation with my app to ensure that it only queries the applicable scope for the different features in my app. If required, I am considering creating an NPM package for my fork as I kind of need this feature urgently, and anyone who needs this implementation can use that package to test before this pull request is merged in.

Note

  1. I don't have any clue how to change a string ('["email", "profile"]') to a valid string[] easily in Java. So do update the code if anyone has a better idea on how to do it
  2. I don't really know if this implementation fits the requirements of feat: add additional scopes request #167
  3. When this pull request is merged in, I can help to update the readme

@reslear
Copy link
Collaborator

reslear commented May 30, 2023

interesting But it needs some fine-tuning.

@AlvinTCH
Copy link
Contributor Author

AlvinTCH commented Jun 2, 2023

Just to add on, I am using this code in production and have no problems with it so far

Do let me know what can be improved on the code whether on readability or others. Would like to learn from it

@reslear
Copy link
Collaborator

reslear commented Jun 2, 2023

please, add example how to use

@AlvinTCH
Copy link
Contributor Author

AlvinTCH commented Jun 6, 2023

we just need to reuse the same code example in the readme.md for web

Example:

import { GoogleAuth } from '@codetrix-studio/capacitor-google-auth';

// use hook after platform dom ready
GoogleAuth.initialize({
  clientId: 'CLIENT_ID.apps.googleusercontent.com',
  scopes: ['profile', 'email'],
  grantOfflineAccess: true,
});

The parameters will be passed to the iOS or Android handlers automatically. So essentially there is no code change for the end user for this to work

@reslear
Copy link
Collaborator

reslear commented Nov 17, 2023

hi @AlvinTCH please sync changes with #321

@reslear
Copy link
Collaborator

reslear commented Nov 26, 2023

I still think your idea is great, I also think you should remove the

  • androidClientId
  • iosClientId

and add an example of how to use it dynamically.

@AlvinTCH could you help with that?

@AlvinTCH
Copy link
Contributor Author

AlvinTCH commented Jan 16, 2024

hi @AlvinTCH please sync changes with #321

synced changes

I still think your idea is great, I also think you should remove the

  • androidClientId
  • iosClientId

and add an example of how to use it dynamically.

@AlvinTCH could you help with that?

For the androidClientId and iosClientId, it is currently in the code base and it might be a huge change that we can roll out in v4 of this package as there might be people using it. We can merge this in first and discuss about v4 in another separate pull request

Sorry I can't see which part that needs explanation due to my own blindsight as it is written by myself. Can you let me know which part of the explanation is not clear on how to use it dynamically?

@reslear

@reslear
Copy link
Collaborator

reslear commented Apr 2, 2024

hi @AlvinTCH let's fix the conflicts then, and I'll do the merge for the next major release

@AlvinTCH
Copy link
Contributor Author

AlvinTCH commented Apr 3, 2024

Ok sure. Let me get to it when I get back tonight

@AlvinTCH
Copy link
Contributor Author

AlvinTCH commented Apr 3, 2024

@reslear it is done

@AlvinTCH
Copy link
Contributor Author

AlvinTCH commented May 1, 2024

by the way I have created an npm package with this particular implementation and have been using it for over a year

@reslear
Copy link
Collaborator

reslear commented May 1, 2024

sounds cool, and you're not using capacitor.config now?

P.s. I plan to merge the changes into this major release

@AlvinTCH
Copy link
Contributor Author

AlvinTCH commented May 1, 2024

Thats a really fast reply. I am still putting the standard values at capacitor.config as a fallback

@reslear reslear merged commit f5fa8d9 into CodetrixStudio:master May 1, 2024
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