Skip to content
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.

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

Decode Base64 usernames/passwords as UTF-8 strings #269

Merged
merged 34 commits into from
Nov 22, 2023

Conversation

RagingKore
Copy link
Contributor

@RagingKore RagingKore commented Oct 16, 2023

Added: Support for UTF8 usernames and passwords
Changed: full cleanup from code format to structure (added .DotSettings)
Changed: created a tests common project to share fixtures and more.
Changed: brand new fixtures and fluent docker abstractions.
Changed: added Shouldy for test assertions.
Changed: added Bogus for creating object fakers.
Changed: removed source generators to provide a path to certs and now simply copy them over.
Changed: refactored client warmup functions.

* fixed incorrect usage of Assert on some tests
* added Shouldy and Bogus to tests
* refactored some tests accordingly
* added minor formatting rules
* upgraded several nuget packages in the test projects
* tests: created new fixture and supporting container components
* tests: global usings for a few tools
@linear
Copy link

linear bot commented Oct 16, 2023

@RagingKore RagingKore self-assigned this Oct 16, 2023
@RagingKore RagingKore added the bug Something isn't working label Oct 16, 2023
* new warmup function attempt :P
* added solution settings
* major cleanup on tests
* code reformat where possible for better readability
* optimized test fixtures.
* reverted spaces to tabs...
* fixed legacy test runners not finding certs
* moved test logger configuration to settings file
* deleted zombie files
@RagingKore
Copy link
Contributor Author

Tests run great locally, but are flaky when running on CI. Having another look at this before making it ready for review again.

@RagingKore RagingKore changed the title Decode Base64 usernames/passwords as UTF-8 strings [DRAFT] Decode Base64 usernames/passwords as UTF-8 strings Nov 2, 2023
@RagingKore RagingKore marked this pull request as draft November 2, 2023 12:38
@RagingKore RagingKore marked this pull request as ready for review November 7, 2023 16:42
@RagingKore RagingKore changed the title [DRAFT] Decode Base64 usernames/passwords as UTF-8 strings Decode Base64 usernames/passwords as UTF-8 strings Nov 7, 2023
@timothycoleman
Copy link
Contributor

Fixed: UserCredentials username and passwords encoding.

Perhaps this should be

Added: Support for UTF8 usernames and passwords

probably best mention utf8 anyway

<GrpcPackageVersion>2.49.0</GrpcPackageVersion>
<GrpcToolsPackageVersion>2.50.0</GrpcToolsPackageVersion>
</PropertyGroup>
<PropertyGroup>
Copy link
Contributor

Choose a reason for hiding this comment

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

did tabs get changed to spaces here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will fix on another PR of "cosmetic" nature. And yes I'm struggling with tabs a lot XD

@@ -0,0 +1,405 @@
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
Copy link
Contributor

Choose a reason for hiding this comment

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

dotsettings and editorconfig are going to conflict, aren't they?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe they will and we can create a new PR to align those.

Copy link
Contributor

@timothycoleman timothycoleman left a comment

Choose a reason for hiding this comment

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

Thanks 👍

For posterity, the fix is in UserCredentials.cs with corresponding tests in UserCredentialsTests.cs

@RagingKore RagingKore merged commit 157b9f6 into master Nov 22, 2023
62 checks passed
@RagingKore RagingKore deleted the sergiosilveira/dev-143 branch November 22, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants