Skip to content
This repository was archived by the owner on May 12, 2026. It is now read-only.

fix: accessToken scopes clean serialization and default as empty list - #1125

Merged
TimurSadykov merged 8 commits into
mainfrom
stim-smbios
Jan 11, 2023
Merged

TimurSadykov merged 8 commits into
mainfrom
stim-smbios

Conversation

@TimurSadykov

@TimurSadykov TimurSadykov commented Jan 7, 2023 •

Copy link
Copy Markdown

Removing extra processing from access token copes serialization
Making empty list of scopes as default, no null

The original change that introduces Scopes into AccessToken is not shipped yet, therefore this change does not change the existing behavior

@TimurSadykov
TimurSadykov requested review from a team and sai-sunder-s January 7, 2023 12:26
@product-auto-label product-auto-label Bot added the size: m Pull request size is medium. label Jan 7, 2023
String scopes =
OAuth2Utils.validateOptionalString(
List<String> scopes =
OAuth2Utils.validateOptionalListString(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

For the existing token with string-typed scopes in the token store, this validation will fail, and they need to refresh to get a new access token?

@TimurSadykov TimurSadykov Jan 9, 2023 •

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The feature is not shipped yet, it was checked in late last year. Basically trying to improve it before it is shipped.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

added this detail to the change description

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This makes sense as we do not need to worry about breaking existing customers.

throw new IOException(
String.format(VALUE_WRONG_TYPE_MESSAGE, errorPrefix, "List<String>", key));
}
return (List<String>) value;

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.

There is no type check for List<String>. Check is for List. What happens if value is something like List<Int>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Should be acceptable as we serialize ourselves. And if the type is wrong its most likely tampered and will fail anyways.

@wangyutongg wangyutongg left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants