Skip to content

Conversation

@mjrousos
Copy link
Member

This helps to ensure users are choosing strong API keys by requiring them to be parsable as GUIDs. Also updates docs to explain the API key requirements and recommend HTTPS for the ASP.NET app to ensure the ASP.NET Core app can request data from it in production scenarios securely.

Fixes #144

@mjrousos mjrousos requested a review from twsouthwick August 24, 2022 20:30
This demonstrates better practice and makes it less likely users will copy the keys directly
@khellang
Copy link
Member

Why force hexadecimal GUIDs? You could shave a lot of extra bytes on the wire by choosing more compact representations using higher-base encodings, while keeping the same amount of entropy.

If the goal is to enforce 128 bits of entropy, could you have a low-level ReadOnlyMemory<byte> (or just byte[]?) property with some convenience methods/propeties to set it using a hexadecimal GUID? Then validate the bytes instead?

@mjrousos
Copy link
Member Author

@khellang yea, there are a lot of options here. I prefer a string over a byte[] since that makes it easier to read from some config sources.

And we could just say "string with minimum 12 characters" or something like that, but forcing the characters to be hex digits makes it less likely users will choose a "bad" password.

I'll re-work this given @twsouthwick's recent changes so that it just validates with a regex data annotation looking for 32 hex characters (with or without dashes or parentheses).

# Conflicts:
#	samples/MvcApp/Web.config
#	src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientExtensions.cs
#	src/Microsoft.AspNetCore.SystemWebAdapters.CoreServices/RemoteAppClientOptions.cs
#	src/Microsoft.AspNetCore.SystemWebAdapters.FrameworkServices/RemoteAppServerExtensions.cs
@mjrousos mjrousos requested a review from twsouthwick August 30, 2022 18:56
@mjrousos mjrousos requested a review from twsouthwick August 30, 2022 19:36
@mjrousos mjrousos merged commit f136fe4 into main Aug 30, 2022
@mjrousos mjrousos deleted the mjrousos/144 branch August 30, 2022 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feedback from review of remote app authentication design

5 participants