PublishAsAzurePostgresFlexibleServer shouldn't require a user and password #2627
Conversation
…sword Allow for PublishAsAzurePostgresFlexibleServer and AsAzurePostgresFlexibleServer to default the user name and password if one isn't provided. Fix dotnet#2389
dotnet-issue-labeler
bot
added
the
area-app-model
| public static string GenerateRandomLettersValue(int minLength) | ||
| { | ||
| ArgumentOutOfRangeException.ThrowIfNegative(minLength); | ||
| ArgumentOutOfRangeException.ThrowIfGreaterThan(minLength, 128); |
There was a problem hiding this comment.
nit, the check in the method above line 41 should be ArgumentOutOfRangeException.ThrowIfGreaterThan(length, 128); not just an assert
There was a problem hiding this comment.
The method above doesn't ever receive public input, which is why it uses an assert.
There was a problem hiding this comment.
Then why throw for negative inputs? But no biggie
| /// <summary> | ||
| /// Creates a random string of upper and lower case letters. | ||
| /// </summary> | ||
| public static string GenerateRandomLettersValue(int minLength) |
There was a problem hiding this comment.
why 'minLength' ? it will be actual length. Method above also produces actual length.
There was a problem hiding this comment.
in fact, it's an odd password generator as it produces a password with a exact number of upper case, exact number of lower case, etc .. not a minimum of each. Hopefully this method will go away as well -- it doesn't seem best practice. Not to mention it should probably enforce an overall minimum eg 10.
There was a problem hiding this comment.
if this is something we can't ship with, should we have an issue on the aspire side as well to ensure we remove/fix it?
There was a problem hiding this comment.
why 'minLength' ?
This was copied from where it is used. Will change to length.
in fact, it's an odd password generator as it produces a password with a exact number of upper case, exact number of lower case, etc .. not a minimum of each. Hopefully this method will go away as well -- it doesn't seem best practice.
I assume you are referring to the existing method. I'm not certain what doesn't seem best practice about it. Can you explain?
Not to mention it should probably enforce an overall minimum eg 10.
All the inputs into the above method are internally hard coded. See #2210. cc @sebastienros.
if this is something we can't ship with
What exactly can't we ship with?
There was a problem hiding this comment.
I've opened #2633 for the Aspire side of Azure/azure-dev#3462
There was a problem hiding this comment.
Perhaps I’m misreading it, but it appears to create a password with exactly the specified number of capital letters, etc rather than “at least that many”. That makes it have less entropy. Eg your bank doesn’t ask you to pick a password with exactly 5 lower case and 5 upper case letters
There was a problem hiding this comment.
Feel free to log an issue. This PR isn't going to address the issue.
Allow for PublishAsAzurePostgresFlexibleServer and AsAzurePostgresFlexibleServer to default the user name and password if one isn't provided.
Fix #2389
Microsoft Reviewers: Open in CodeFlow