Skip to content

Add OAuth credentials object for v2 Slack app#199

Merged
zmarushchak-hs merged 5 commits intomasterfrom
add-v2-oauth-creds
Oct 20, 2020
Merged

Add OAuth credentials object for v2 Slack app#199
zmarushchak-hs merged 5 commits intomasterfrom
add-v2-oauth-creds

Conversation

@zmarushchak-hs
Copy link
Copy Markdown
Contributor

Object added in order to be able to deserialize response for oauth.v2.access request which is used on Slack app installation to exchange temporary OAuth code for an API access token.

Copy link
Copy Markdown
Contributor

@omotnyk omotnyk left a comment

Choose a reason for hiding this comment

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

We heavily rely on bot user ID. Should we store it? Also, it seems like this changes the auth process completely. As far as I see, https://api.slack.com/methods/oauth.v2.access still returns both user and bot token but this model doesn't store user token. Is this intentional? Will we ever need this token again? right now we do a lot of things using user token. Will the migration process seamless in this case?

public abstract String getAccessToken();

@JsonProperty("team")
public abstract SlackTeamLite getSlackTeamLite();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this can be just getTeam. This way we don't need to override json property name.

@HubSpotStyle
@JsonNaming(SnakeCaseStrategy.class)
public abstract class OAuthV2CredentialsIF {
public abstract String getAccessToken();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wonder if we should rename this to getBotAccessToken.

@zmarushchak-hs zmarushchak-hs merged commit 388cb37 into master Oct 20, 2020
@zmarushchak-hs zmarushchak-hs deleted the add-v2-oauth-creds branch October 20, 2020 15:04
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.

2 participants