Skip to content

Commit

Permalink
Apply review suggestions on KOOK provider
Browse files Browse the repository at this point in the history
Removed IsBot, IsOnline, IsBanned claims
Updated unit tests reflecting the changes of claims
Fixed incorrect naming in project description
Optimized documentation around default scopes

Reviewed-by: martincostello
  • Loading branch information
gehongyan committed Jan 5, 2023
1 parent b46a91f commit fcafb83
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 13 deletions.
4 changes: 2 additions & 2 deletions docs/kook.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ services.AddAuthentication(options => /* Auth configuration */)

// The scope get_user_info is added by default
// Please make sure the scope get_user_info is enabled in your KOOK developer center
// Hence, whether to add the scope get_user_info here manually is optional
// options.Scope.Add("get_user_info");
// If you do not want to use the default scope indeed, try remove it by
// options.Scope.Remove("get_user_info");
});
```

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
<!-- TODO Remove once published to NuGet.org -->
<PropertyGroup>
<DisablePackageBaselineValidation>true</DisablePackageBaselineValidation>
<PackageValidationBaselineVersion>7.0.1</PackageValidationBaselineVersion>
</PropertyGroup>

<PropertyGroup>
<Description>ASP.NET Core security middleware enabling Kook authentication.</Description>
<Description>ASP.NET Core security middleware enabling KOOK authentication.</Description>
<Authors>Gehongyan</Authors>
<PackageTags>aspnetcore;authentication;kaiheila;kook;kookapp;oauth;security</PackageTags>
</PropertyGroup>
Expand Down
3 changes: 0 additions & 3 deletions src/AspNet.Security.OAuth.Kook/KookAuthenticationConstants.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@ public static class KookAuthenticationConstants
{
public static class Claims
{
public const string IsOnline = "urn:kook:user:is_online";
public const string IsBanned = "urn:kook:user:is_banned";
public const string OperatingSystem = "urn:kook:user:operating_system";
public const string AvatarUrl = "urn:kook:user:avatar:url";
public const string BannerUrl = "urn:kook:user:banner:url";
public const string IsBot = "urn:kook:user:is_bot";
public const string IsMobileVerified = "urn:kook:user:is_mobile_verified";
public const string IdentifyNumber = "urn:kook:user:identify_num";
}
Expand Down
2 changes: 0 additions & 2 deletions src/AspNet.Security.OAuth.Kook/KookAuthenticationHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,6 @@ protected override async Task<AuthenticationTicket> CreateTicketAsync(
var principal = new ClaimsPrincipal(identity);
var user = payload.RootElement.GetProperty("data");

identity.AddClaim(new Claim(KookAuthenticationConstants.Claims.IsBanned, (user.GetProperty("status").GetInt32() == 10).ToString()));

var context = new OAuthCreatingTicketContext(principal, properties, Context, Scheme, Options, Backchannel, tokens, user);
context.RunClaimActions();

Expand Down
2 changes: 0 additions & 2 deletions src/AspNet.Security.OAuth.Kook/KookAuthenticationOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,9 @@ public KookAuthenticationOptions()
ClaimActions.MapJsonKey(ClaimTypes.Name, "username");
ClaimActions.MapJsonKey(ClaimTypes.MobilePhone, "mobile");
ClaimActions.MapJsonKey(KookAuthenticationConstants.Claims.IdentifyNumber, "identify_num");
ClaimActions.MapJsonKey(KookAuthenticationConstants.Claims.IsOnline, "online");
ClaimActions.MapJsonKey(KookAuthenticationConstants.Claims.OperatingSystem, "os");
ClaimActions.MapJsonKey(KookAuthenticationConstants.Claims.AvatarUrl, "avatar");
ClaimActions.MapJsonKey(KookAuthenticationConstants.Claims.BannerUrl, "banner");
ClaimActions.MapJsonKey(KookAuthenticationConstants.Claims.IsBot, "bot");
ClaimActions.MapJsonKey(KookAuthenticationConstants.Claims.IsMobileVerified, "mobile_verified");

Scope.Add("get_user_info");
Expand Down
3 changes: 0 additions & 3 deletions test/AspNet.Security.OAuth.Providers.Tests/Kook/KookTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,9 @@ protected internal override void RegisterAuthentication(AuthenticationBuilder bu
[InlineData(ClaimTypes.Name, "test")]
[InlineData(ClaimTypes.MobilePhone, "110****2333")]
[InlineData(Claims.IdentifyNumber, "1670")]
[InlineData(Claims.IsOnline, "False")]
[InlineData(Claims.IsBanned, "False")]
[InlineData(Claims.OperatingSystem, "iOS")]
[InlineData(Claims.AvatarUrl, "https://xxx.com/assets/avatar.png/icon")]
[InlineData(Claims.BannerUrl, "https://xxx.com/assets/banner.png/icon")]
[InlineData(Claims.IsBot, "True")]
[InlineData(Claims.IsMobileVerified, "True")]
public async Task Can_Sign_In_Using_Kook(string claimType, string claimValue)
{
Expand Down

0 comments on commit fcafb83

Please sign in to comment.