✨ Steam - Load additional claims from user information endpoint #166
Conversation
|
|
||
|
|
||
|
|
||
| return; |
There was a problem hiding this comment.
Redundant.
| return; |
|
|
||
| return; | ||
|
|
||
| void AddSteamProfileClaim(string claimType) |
There was a problem hiding this comment.
Not a fan of the doubly-nested local method here.
| { | ||
| if (profile.TryGetProperty(claimType, out var claimValue)) | ||
| { | ||
| identity.AddClaim(new Claim(claimType, claimValue.GetRawText().Trim('"'), ClaimValueTypes.String, Options.ClaimsIssuer)); |
There was a problem hiding this comment.
I'm not sure about this mangling of the extra properties here. We should just deal with ones that are strings and leave others to the user to manually configure.
Alternatively, parse each one appropriately from the type it is in the JSON to match whatever it's documented to be.
| /// </summary> | ||
| public static class SteamClaimTypes | ||
| { | ||
| // Public |
There was a problem hiding this comment.
Redundant given the next line immediately has a public access modifier.
| // Public |
| public const string AvatarMedium = "avatarmedium"; | ||
| public const string AvatarFull = "avatarfull"; | ||
| /// <summary> | ||
| /// The user's current status. 0 - Offline, 1 - Online, 2 - Busy, 3 - Away, 4 - Snooze, 5 - looking to trade, 6 - looking to play. If the player's profile is private, this will always be "0", except is the user has set their status to looking to trade or looking to play, because a bug makes those status appear even if the profile is private. |
There was a problem hiding this comment.
Remove the values. We don't need to parrot Steam's own documentation.
| public const string LastLogOff = "lastlogoff"; | ||
| public const string CommentPermission = "commentpermission"; | ||
|
|
||
| // Private |
There was a problem hiding this comment.
If they're private to Steam we shouldn't be depending on them in our code. If you mean they're an implementation detail of the implementation, then they should be declared as ˋinternal`.
Summarize the changes this Pull Request makes.
Load additional claims from user information endpoint
If adding a new provider, please ensure you have included appropriate test(s).
There's a few different ways to test this, let me know how/if you want this tested.
Please include a reference to a GitHub issue if appropriate.