Skip to content
This repository has been archived by the owner on Dec 24, 2020. It is now read-only.

Commit

Permalink
Use JwtSecurityTokenHandler.CreateEncodedJwt() instead of CreateToken…
Browse files Browse the repository at this point in the history 
…()/WriteToken() to ensure the resulting token is not signed twice
  • Loading branch information
@kevinchalet
kevinchalet committed Jul 30, 2017
1 parent 3073ceb commit a499f11
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 35 deletions.
32 changes: 22 additions & 10 deletions src/AspNet.Security.OpenIdConnect.Server/OpenIdConnectServerHandler.Serialization.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

using System;
using System.IdentityModel.Tokens.Jwt;
using System.Linq;
using System.Security.Claims;
using System.Text;
Expand Down Expand Up @@ -237,23 +238,36 @@ private async Task<string> SerializeAccessTokenAsync(
break;
}

var token = notification.SecurityTokenHandler.CreateToken(new SecurityTokenDescriptor
var descriptor = new SecurityTokenDescriptor
{
Subject = identity,
Issuer = notification.Issuer,
SigningCredentials = notification.SigningCredentials,
IssuedAt = notification.Ticket.Properties.IssuedUtc?.UtcDateTime,
NotBefore = notification.Ticket.Properties.IssuedUtc?.UtcDateTime,
Expires = notification.Ticket.Properties.ExpiresUtc?.UtcDateTime
});
};

string token;

// When the token handler is a JWT token handler, directly use JwtSecurityTokenHandler.CreateEncodedJwt()
// instead of calling JwtSecurityTokenHandler.WriteToken() to avoid signing the token twice.
if (notification.SecurityTokenHandler is JwtSecurityTokenHandler jwtSecurityTokenHandler)
{
token = jwtSecurityTokenHandler.CreateEncodedJwt(descriptor);
}

var result = notification.SecurityTokenHandler.WriteToken(token);
else
{
token = notification.SecurityTokenHandler.WriteToken(
notification.SecurityTokenHandler.CreateToken(descriptor));
}

Logger.LogTrace("A new access token was successfully generated using the specified " +
"security token handler: {Token} ; {Claims} ; {Properties}.",
result, ticket.Principal.Claims, ticket.Properties.Items);
token, ticket.Principal.Claims, ticket.Properties.Items);

return result;
return token;
}

private async Task<string> SerializeIdentityTokenAsync(
Expand Down Expand Up @@ -439,7 +453,7 @@ private async Task<string> SerializeIdentityTokenAsync(
break;
}

var token = notification.SecurityTokenHandler.CreateToken(new SecurityTokenDescriptor
var token = notification.SecurityTokenHandler.CreateEncodedJwt(new SecurityTokenDescriptor
{
Subject = identity,
Issuer = notification.Issuer,
Expand All @@ -449,13 +463,11 @@ private async Task<string> SerializeIdentityTokenAsync(
Expires = notification.Ticket.Properties.ExpiresUtc?.UtcDateTime
});

var result = notification.SecurityTokenHandler.WriteToken(token);

Logger.LogTrace("A new identity token was successfully generated using the specified " +
"security token handler: {Token} ; {Claims} ; {Properties}.",
result, ticket.Principal.Claims, ticket.Properties.Items);
token, ticket.Principal.Claims, ticket.Properties.Items);

return result;
return token;
}

private async Task<string> SerializeRefreshTokenAsync(
Expand Down
8 changes: 6 additions & 2 deletions src/Owin.Security.OpenIdConnect.Server/OpenIdConnectServerHandler.Serialization.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,11 @@ private async Task<string> SerializeAccessTokenAsync(
notification.Ticket.Properties.ExpiresUtc?.UtcDateTime)
});

var result = notification.SecurityTokenHandler.WriteToken(token);
// When the access token is a JWT token, directly use RawData instead of calling
// JwtSecurityTokenHandler.WriteToken(token) to avoid signing the token twice.
var result = token is JwtSecurityToken jwtSecurityToken ?
jwtSecurityToken.RawData :
notification.SecurityTokenHandler.WriteToken(token);

Logger.LogTrace("A new access token was successfully generated using the specified " +
"security token handler: {Token} ; {Claims} ; {Properties}.",
Expand Down Expand Up @@ -442,7 +446,7 @@ private async Task<string> SerializeIdentityTokenAsync(
notification.Ticket.Properties.ExpiresUtc?.UtcDateTime)
});

var result = notification.SecurityTokenHandler.WriteToken(token);
var result = ((JwtSecurityToken) token).RawData;

Logger.LogTrace("A new identity token was successfully generated using the specified " +
"security token handler: {Token} ; {Claims} ; {Properties}.",
Expand Down
20 changes: 4 additions & 16 deletions ...pNet.Security.OpenIdConnect.Server.Tests/OpenIdConnectServerHandlerTests.Serialization.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1220,14 +1220,8 @@ public async Task SerializeAccessTokenAsync_MissingSigningCredentialsCauseAnExce
public async Task SerializeAccessTokenAsync_UsesAccessTokenHandlerWhenRegistered()
{
// Arrange
var token = Mock.Of<SecurityToken>();

var format = new Mock<JwtSecurityTokenHandler>();
format.Setup(mock => mock.CreateToken(It.IsAny<SecurityTokenDescriptor>()))
.Returns(token)
.Verifiable();

format.Setup(mock => mock.WriteToken(token))
format.Setup(mock => mock.CreateEncodedJwt(It.IsAny<SecurityTokenDescriptor>()))
.Returns("7F82F1A3-8C9F-489F-B838-4B644B7C92B2")
.Verifiable();

Expand Down Expand Up @@ -1265,8 +1259,7 @@ public async Task SerializeAccessTokenAsync_UsesAccessTokenHandlerWhenRegistered

// Assert
Assert.Equal("7F82F1A3-8C9F-489F-B838-4B644B7C92B2", response.AccessToken);
format.Verify(mock => mock.CreateToken(It.IsAny<SecurityTokenDescriptor>()), Times.Once());
format.Verify(mock => mock.WriteToken(token), Times.Once());
format.Verify(mock => mock.CreateEncodedJwt(It.IsAny<SecurityTokenDescriptor>()), Times.Once());
}

[Fact]
Expand Down Expand Up @@ -1936,11 +1929,7 @@ public async Task SerializeIdentityTokenAsync_UsesIdentityTokenHandler()
{
// Arrange
var format = new Mock<JwtSecurityTokenHandler>();
format.Setup(mock => mock.CreateToken(It.IsAny<SecurityTokenDescriptor>()))
.Returns(It.IsAny<SecurityToken>())
.Verifiable();

format.Setup(mock => mock.WriteToken(It.IsAny<SecurityToken>()))
format.Setup(mock => mock.CreateEncodedJwt(It.IsAny<SecurityTokenDescriptor>()))
.Returns("7F82F1A3-8C9F-489F-B838-4B644B7C92B2")
.Verifiable();

Expand Down Expand Up @@ -1979,8 +1968,7 @@ public async Task SerializeIdentityTokenAsync_UsesIdentityTokenHandler()

// Assert
Assert.Equal("7F82F1A3-8C9F-489F-B838-4B644B7C92B2", response.IdToken);
format.Verify(mock => mock.CreateToken(It.IsAny<SecurityTokenDescriptor>()), Times.Once());
format.Verify(mock => mock.WriteToken(It.IsAny<SecurityToken>()), Times.Once());
format.Verify(mock => mock.CreateEncodedJwt(It.IsAny<SecurityTokenDescriptor>()), Times.Once());
}

[Fact]
Expand Down
21 changes: 14 additions & 7 deletions ...Owin.Security.OpenIdConnect.Server.Tests/OpenIdConnectServerHandlerTests.Serialization.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1774,13 +1774,21 @@ public async Task SerializeIdentityTokenAsync_MissingSigningCredentialsCauseAnEx
public async Task SerializeIdentityTokenAsync_UsesIdentityTokenHandler()
{
// Arrange
var token = new JwtSecurityToken(
"eyJhbGciOiJSUzI1NiIsImtpZCI6IjFlOWdkazcifQ.ewogImlzc" +
"yI6ICJodHRwOi8vc2VydmVyLmV4YW1wbGUuY29tIiwKICJzdWIiOiAiMjQ4Mjg5" +
"NzYxMDAxIiwKICJhdWQiOiAiczZCaGRSa3F0MyIsCiAibm9uY2UiOiAibi0wUzZ" +
"fV3pBMk1qIiwKICJleHAiOiAxMzExMjgxOTcwLAogImlhdCI6IDEzMTEyODA5Nz" +
"AKfQ.ggW8hZ1EuVLuxNuuIJKX_V8a_OMXzR0EHR9R6jgdqrOOF4daGU96Sr_P6q" +
"Jp6IcmD3HP99Obi1PRs-cwh3LO-p146waJ8IhehcwL7F09JdijmBqkvPeB2T9CJ" +
"NqeGpe-gccMg4vfKjkM8FcGvnzZUN4_KSP0aAp1tOJ1zZwgjxqGByKHiOtX7Tpd" +
"QyHE5lcMiKPXfEIQILVq0pc_E2DzL7emopWoaoZTF_m0_N0YzFC6g6EJbOEoRoS" +
"K5hoDalrcvRYLSrQAZZKflyuVCyixEoV9GfNQC3_osjzw2PAithfubEEBLuVVk4" +
"XUVrWOLrLl0nx7RkKU8NXNHq-rvKMzqg");

var format = new Mock<JwtSecurityTokenHandler>();
format.Setup(mock => mock.CreateToken(It.IsAny<SecurityTokenDescriptor>()))
.Returns(It.IsAny<SecurityToken>())
.Verifiable();

format.Setup(mock => mock.WriteToken(It.IsAny<SecurityToken>()))
.Returns("7F82F1A3-8C9F-489F-B838-4B644B7C92B2")
.Returns(token)
.Verifiable();

var server = CreateAuthorizationServer(options =>
Expand Down Expand Up @@ -1817,9 +1825,8 @@ public async Task SerializeIdentityTokenAsync_UsesIdentityTokenHandler()
});

// Assert
Assert.Equal("7F82F1A3-8C9F-489F-B838-4B644B7C92B2", response.IdToken);
Assert.Equal(token.RawData, response.IdToken);
format.Verify(mock => mock.CreateToken(It.IsAny<SecurityTokenDescriptor>()), Times.Once());
format.Verify(mock => mock.WriteToken(It.IsAny<SecurityToken>()), Times.Once());
}

[Fact]
Expand Down

0 comments on commit a499f11

Please sign in to comment.