Skip to content

Improve missing/malformed file handling in user-jwts #42309

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jun 21, 2022
Merged

Improve missing/malformed file handling in user-jwts #42309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
135e808
Improve missing/malformed file handling in user-jwts
captainsafia Jun 20, 2022
9ff265e
Address feedback from review
captainsafia Jun 20, 2022
5b7494d
Update src/Tools/dotnet-user-jwts/src/Helpers/DevJwtCliHelpers.cs
captainsafia Jun 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Tools/dotnet-user-jwts/src/Commands/CreateCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ private static (JwtCreatorOptions, bool, string) ValidateArguments(
var name = nameOption.HasValue() ? nameOption.Value() : Environment.UserName;
optionsString += $"{Resources.JwtPrint_Name}: {name}{Environment.NewLine}";

var audience = audienceOption.HasValue() ? audienceOption.Values : DevJwtCliHelpers.GetAudienceCandidatesFromLaunchSettings(project).ToList();
optionsString += audienceOption.HasValue() ? $"{Resources.JwtPrint_Audiences}: {audience}{Environment.NewLine}" : string.Empty;
if (audience is null)
var audience = audienceOption.HasValue() ? audienceOption.Values : DevJwtCliHelpers.GetAudienceCandidatesFromLaunchSettings(project);
optionsString += audienceOption.HasValue() ? $"{Resources.JwtPrint_Audiences}: {string.Join(", ", audience)}{Environment.NewLine}" : string.Empty;
if (audience is null || audience.Count == 0)
{
reporter.Error(Resources.CreateCommand_NoAudience_Error);
isValid = false;
Expand Down
81 changes: 61 additions & 20 deletions src/Tools/dotnet-user-jwts/src/Helpers/DevJwtCliHelpers.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.IdentityModel.Tokens.Jwt;
using System.Linq;
using System.Text.Json;
using System.Text.Json.Nodes;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.Configuration.UserSecrets;
using Microsoft.Extensions.Tools.Internal;
Expand Down Expand Up @@ -84,35 +85,36 @@ public static byte[] CreateSigningKeyMaterial(string userSecretsId, bool reset =
var secretsFilePath = PathHelper.GetSecretsPathFromSecretsId(userSecretsId);
Directory.CreateDirectory(Path.GetDirectoryName(secretsFilePath));

IDictionary<string, string> secrets = null;
JsonObject secrets = null;
if (File.Exists(secretsFilePath))
{
using var secretsFileStream = new FileStream(secretsFilePath, FileMode.Open, FileAccess.Read);
if (secretsFileStream.Length > 0)
{
secrets = JsonSerializer.Deserialize<IDictionary<string, string>>(secretsFileStream) ?? new Dictionary<string, string>();
secrets = JsonSerializer.Deserialize<JsonObject>(secretsFileStream);
}
}

secrets ??= new Dictionary<string, string>();
secrets ??= new JsonObject();

if (reset && secrets.ContainsKey(DevJwtsDefaults.SigningKeyConfigurationKey))
{
secrets.Remove(DevJwtsDefaults.SigningKeyConfigurationKey);
}
secrets.Add(DevJwtsDefaults.SigningKeyConfigurationKey, Convert.ToBase64String(newKeyMaterial));
secrets.Add(DevJwtsDefaults.SigningKeyConfigurationKey, JsonValue.Create(Convert.ToBase64String(newKeyMaterial)));

using var secretsWriteStream = new FileStream(secretsFilePath, FileMode.Create, FileAccess.Write);
JsonSerializer.Serialize(secretsWriteStream, secrets);

return newKeyMaterial;
}

public static string[] GetAudienceCandidatesFromLaunchSettings(string project)
public static List<string> GetAudienceCandidatesFromLaunchSettings(string project)
{
ArgumentException.ThrowIfNullOrEmpty(nameof(project));

var launchSettingsFilePath = Path.Combine(Path.GetDirectoryName(project)!, "Properties", "launchSettings.json");
var applicationUrls = new List<string>();
if (File.Exists(launchSettingsFilePath))
{
using var launchSettingsFileStream = new FileStream(launchSettingsFilePath, FileMode.Open, FileAccess.Read);
Expand All @@ -124,26 +126,65 @@ public static string[] GetAudienceCandidatesFromLaunchSettings(string project)
var profilesEnumerator = profiles.EnumerateObject();
foreach (var profile in profilesEnumerator)
{
if (profile.Value.TryGetProperty("commandName", out var commandName))
if (ExtractKestrelUrlsFromProfile(profile) is { } kestrelUrls)
{
if (commandName.ValueEquals("Project"))
{
if (profile.Value.TryGetProperty("applicationUrl", out var applicationUrl))
{
var value = applicationUrl.GetString();
if (value is { } applicationUrls)
{
return applicationUrls.Split(';');
}
}
}
applicationUrls.AddRange(kestrelUrls);
}

Copy link
Member

@BrennanConroy BrennanConroy Jun 21, 2022

Choose a reason for hiding this comment

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

nit: else if

Copy link
Member Author

@captainsafia captainsafia Jun 21, 2022

Choose a reason for hiding this comment

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

I don't think that's the right call here. We want to populate audience from both IIS and Kestrel-based URLs here so we get a complete set of audiences by the end.

Copy link
Member

@BrennanConroy BrennanConroy Jun 21, 2022

Choose a reason for hiding this comment

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

The reason I think it should be else if is because if the Kestrel check of profile.Value.TryGetProperty("commandName", out var commandName) passes then the IIS check won't pass for this specific profile. Again, it's a nit, not code we need to worry about performance for.

if (ExtractIISExpressUrlFromProfile(profile) is { } iisUrls)
{
applicationUrls.AddRange(iisUrls);
}
}
}
}
}

return null;
return applicationUrls;

static List<string> ExtractIISExpressUrlFromProfile(JsonProperty profile)
{
if (profile.NameEquals("iisSettings"))
{
if (profile.Value.TryGetProperty("iisExpress", out var iisExpress))
{
List<string> iisUrls = new();
if (iisExpress.TryGetProperty("applicationUrl", out var iisUrl))
{
iisUrls.Add(iisUrl.GetString());
Copy link
Member

@BrennanConroy BrennanConroy Jun 21, 2022

Choose a reason for hiding this comment

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

can these urls also be ; separated?

Copy link
Member

@DamianEdwards DamianEdwards Jun 21, 2022

Choose a reason for hiding this comment

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

I didn't think so, but we'd need to check with VS

Copy link
Member Author

@captainsafia captainsafia Jun 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I tested it out. VS will throw if you have something like this:

"iisExpress": {
    "applicationUrl": "http://localhost:22686;http://localhost:22687",
    "sslPort": 44333
  }

}

if (iisExpress.TryGetProperty("sslPort", out var sslPort))
{
iisUrls.Add($"https://localhost:{sslPort.GetInt32()}");
}

return iisUrls;
}
}

return null;
}

static string[] ExtractKestrelUrlsFromProfile(JsonProperty profile)
{
if (profile.Value.TryGetProperty("commandName", out var commandName))
{
if (commandName.ValueEquals("Project"))
{
if (profile.Value.TryGetProperty("applicationUrl", out var applicationUrl))
{
var value = applicationUrl.GetString();
if (value is { } urls)
{
return urls.Split(';');
}
}
}
}

return null;
}
}

public static void PrintJwt(IReporter reporter, Jwt jwt, bool showAll, JwtSecurityToken fullToken = null)
Expand All @@ -163,12 +204,12 @@ public static void PrintJwt(IReporter reporter, Jwt jwt, bool showAll, JwtSecuri
: string.Join(", ", jwt.Scopes);
reporter.Output($"{Resources.JwtPrint_Scopes}: {scopesValue}");
}

if (!jwt.Roles.IsNullOrEmpty() || showAll)
{
var rolesValue = jwt.Roles.IsNullOrEmpty()
? "none"
: String.Join(", ", jwt.Roles);
: string.Join(", ", jwt.Roles);
reporter.Output($"{Resources.JwtPrint_Roles}: [{rolesValue}]");
}

Expand Down
58 changes: 29 additions & 29 deletions src/Tools/dotnet-user-jwts/src/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<root>
<!--
Microsoft ResX Schema
<!--
Microsoft ResX Schema

Version 2.0
The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes

The primary goals of this format is to allow a simple XML format
that is mostly human readable. The generation and parsing of the
various data types are done through the TypeConverter classes
associated with the data types.

Example:

... ado.net/XML headers & schema ...
<resheader name="resmimetype">text/microsoft-resx</resheader>
<resheader name="version">2.0</resheader>
Expand All @@ -26,36 +26,36 @@
<value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
<comment>This is a comment</comment>
</data>
There are any number of "resheader" rows that contain simple

There are any number of "resheader" rows that contain simple
name/value pairs.
Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the

Each data row contains a name, and value. The row also contains a
type or mimetype. Type corresponds to a .NET class that support
text/value conversion through the TypeConverter architecture.
Classes that don't support this are serialized and stored with the
mimetype set.
The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not

The mimetype is used for serialized objects, and tells the
ResXResourceReader how to depersist the object. This is currently not
extensible. For a given mimetype the value must be set accordingly:
Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can

Note - application/x-microsoft.net.object.binary.base64 is the format
that the ResXResourceWriter will generate, however the reader can
read any of the formats listed below.

mimetype: application/x-microsoft.net.object.binary.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.soap.base64
value : The object must be serialized with
value : The object must be serialized with
: System.Runtime.Serialization.Formatters.Soap.SoapFormatter
: and then encoded with base64 encoding.

mimetype: application/x-microsoft.net.object.bytearray.base64
value : The object must be serialized into a byte array
value : The object must be serialized into a byte array
: using a System.ComponentModel.TypeConverter
: and then encoded with base64 encoding.
-->
Expand Down Expand Up @@ -169,7 +169,7 @@
<value>The name of the user to create the JWT for. Defaults to the current environment user.</value>
</data>
<data name="CreateCommand_NoAudience_Error" xml:space="preserve">
<value>Could not determine the project's HTTPS URL. Please specify an audience for the JWT using the --audience option.</value>
<value>Could not determine the project's URL. Please specify an audience for the JWT using the --audience option or ensure that launchSettings.json is configured properly.</value>
</data>
<data name="CreateCommand_NotBeforeOption_Description" xml:space="preserve">
<value>The UTC date &amp; time the JWT should not be valid before in the format 'yyyy-MM-dd [[HH:mm[[:ss]]]]'. Defaults to the date &amp; time the JWT is created.</value>
Expand Down Expand Up @@ -303,4 +303,4 @@
<data name="RemoveCommand_NoJwtFound" xml:space="preserve">
<value>No JWT with ID '{0}' found.</value>
</data>
</root>
</root>
6 changes: 5 additions & 1 deletion src/Tools/dotnet-user-jwts/src/dotnet-user-jwts.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,8 @@
<Reference Include="Microsoft.Extensions.Configuration" />
<Reference Include="Microsoft.Extensions.Configuration.UserSecrets" />
</ItemGroup>
</Project>

<ItemGroup>
<InternalsVisibleTo Include="Microsoft.AspNetCore.Authentication.JwtBearer.Tools.Tests" />
</ItemGroup>
</Project>
12 changes: 10 additions & 2 deletions src/Tools/dotnet-user-jwts/test/UserJwtsTestFixture.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ namespace Microsoft.AspNetCore.Authentication.JwtBearer.Tools.Tests;
public class UserJwtsTestFixture : IDisposable
{
private Stack<Action> _disposables = new Stack<Action>();
private string TestSecretsId = Guid.NewGuid().ToString();
internal string TestSecretsId = Guid.NewGuid().ToString();

private const string ProjectTemplate = @"<Project Sdk=""Microsoft.NET.Sdk"">
<PropertyGroup>
Expand All @@ -26,6 +26,14 @@ public class UserJwtsTestFixture : IDisposable
private const string LaunchSettingsTemplate = @"
{
""profiles"": {
""iisSettings"": {
""windowsAuthentication"": false,
""anonymousAuthentication"": true,
""iisExpress"": {
""applicationUrl"": ""http://localhost:23528"",
""sslPort"": 44395
}
},
""HttpApiSampleApp"": {
""commandName"": ""Project"",
""dotnetRunMessages"": true,
Expand Down Expand Up @@ -71,7 +79,7 @@ public string CreateProject(bool hasSecret = true)
catch { }
});
}

_disposables.Push(() => TryDelete(projectPath.FullName));

return projectPath.FullName;
Expand Down
Loading