Android CODE Auth Flow #121
Conversation
- some cleanup in java auth module - update to babel config for example - update to ApiConfig to include authType - update to spotifySession to include code
- Provide button for new auth type
| @@ -48,15 +48,19 @@ public void authorize(ReadableMap config, Promise promise) { | |||
| String redirectUri = mConfig.getString("redirectURL"); | |||
| Boolean showDialog = mConfig.getBoolean("showDialog"); | |||
| String[] scopes = convertScopes(mConfig); | |||
| AuthorizationResponse.Type responseType = mConfig.hasKey("authType") ? | |||
There was a problem hiding this comment.
@MrBuggySan Thanks for the idea for cleaning up this code.
| * @type {string} | ||
| * @memberof SpotifySession | ||
| */ | ||
| code?: string; |
There was a problem hiding this comment.
@reteps would you have a quick gander at the changes? Also, I've updated the example app.. but I'm currently using the CODE / TOKEN interchangeably. What is the expected behavior of the CODE type?
You were mentioning being able to skip the auth screen. When I use the code type it still does bring up the spinner for a moment. It seems that either the accessToken or the code can be used to connect to the spotify remote. So I think I can probably remove this from the API and just return either the code or the token in the accessToken property.
Anyways, I can't find any docs on the CODE option so I'm not sure what the expectation is.
There was a problem hiding this comment.
This is a bad coding practice. CODE is an auth flow for different needs (e.g. when you need to refresh tokens on your server), and inserting both code and accessToken in one accessToken property made me think this feature is actually not working / not supported. After an hour of research, I luckily found this PR. The only way I could make sure the CODE flow is working is by comparing the length of the accessToken field (code appears to be longer, I had an example).
Grateful for your work on the library, but please don't use one field for two completely different things.
| @@ -146,12 +148,13 @@ class AppContextProvider extends React.Component<{}, AppContextState> { | |||
| })); | |||
|
|
|||
| // Initialize the session | |||
| const { accessToken } = await auth.authorize(config); | |||
| const { accessToken, code } = await auth.authorize(config); | |||
| const token = code || accessToken; | |||
There was a problem hiding this comment.
@reteps Both code and accessToken seem to work to connect to the remote, but perhaps Im misunderstanding the purpose of each.
|
Thanks @uptotec for the original PR. |
This PR just addresses all of the comments from the original PR #68.
Created a new branch so I could do the code changes directly.