Conversation
| protected static string _loadUser(string text) | ||
| { | ||
| StringReader reader = null; | ||
| try |
There was a problem hiding this comment.
we could use using here but I see it's lifted from the original file, so better to leave as is.
sixlettervariables
left a comment
sixlettervariables
left a comment
There was a problem hiding this comment.
Minor nits; also double checked we wouldn't introduce any issues with the new base class.
| string text = secure.ToString(); | ||
|
|
||
| // if it's a nk file, it only has the nkey | ||
| if (text.StartsWith("SU")) | ||
| { | ||
| return Nkeys.FromSeed(text); | ||
| } | ||
|
|
||
| // otherwise assume it's a creds file. | ||
| reader = new StringReader(text); |
There was a problem hiding this comment.
Not sure why the original code had the ToString
| string text = secure.ToString(); | |
| // if it's a nk file, it only has the nkey | |
| if (text.StartsWith("SU")) | |
| { | |
| return Nkeys.FromSeed(text); | |
| } | |
| // otherwise assume it's a creds file. | |
| reader = new StringReader(text); | |
| // if it's a nk file, it only has the nkey | |
| if (secure.StartsWith("SU")) | |
| { | |
| return Nkeys.FromSeed(secure); | |
| } | |
| // otherwise assume it's a creds file. | |
| reader = new StringReader(secure); |
There was a problem hiding this comment.
leftover from when I was trying secure. Will fix.
| /// utility methods to read a private seed or user JWT. | ||
| /// </summary> | ||
| public class DefaultUserJWTHandler | ||
| public class DefaultUserJWTHandler : BaseUserJWTHandler |
There was a problem hiding this comment.
Since we had a breaking change earlier I went and double checked this would not introduce any issues, and I don't believe it will:
❓ REQUIRES JUDGMENT: Introducing a new base class
A type can be introduced into a hierarchy between two existing types if it doesn't introduce any new abstract members or change the semantics or behavior of existing types. For example, in .NET Framework 2.0, the DbConnection class became a new base class for SqlConnection, which had previously derived directly from Component.
There was a problem hiding this comment.
I can change from base to a delegate
| reader?.Dispose(); | ||
| throw new NATSException("Credentials file does not contain a JWT"); | ||
| } | ||
| return user.ToString(); |
There was a problem hiding this comment.
| return user.ToString(); | |
| return user; |
There was a problem hiding this comment.
yeah, that was left over from trying secure.
| { | ||
| Nkeys.Wipe(line); | ||
| Nkeys.Wipe(text); | ||
| Nkeys.Wipe(seed); |
There was a problem hiding this comment.
FYI, these were removed because the wipe tries to wipe a string which is not possible. The wipe method was recently changed to a no-op.
3 participants
No description provided.