-
Notifications
You must be signed in to change notification settings - Fork 43
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
Allow using DOMAIN COMPONENT for AD DS certificates #75
Closed
Closed
Changes from 4 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
6080ea5
Create pull.yml
alfonsrv 7e955fa
Allow using DOMAIN COMPONENT for AD DS certificates
alfonsrv defd072
Improved sorting for more complicated patterns
alfonsrv b6a176c
USE_TZ compatibility
alfonsrv 38c6e4f
Fix overly complex Django admin widget
alfonsrv be00b58
sign_cert management command different expiring logic
alfonsrv bad161d
Fixing otherName UTF8 encoding
alfonsrv 082ccd3
Add documentation on custom extensions
alfonsrv 83619b2
Adding tests x minor code-base alignments
alfonsrv d4fa7b6
Merge branch 'mathiasertl:master' into django-dev
alfonsrv 0c9dec8
Migrations
alfonsrv a0b8bc4
Move DC above OU, docs header fix
alfonsrv 446b050
Merge branch 'mathiasertl:master' into django-dev
alfonsrv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function here: I think I understand what your updated comment means to express, but the code is far from obvious. And I'm frankly surprised that the old code does not respect sort stability.
subject[::-1]
is just reversing the list. At least a simple test did that. If yes, then use the more obviousreversed()
instead.if
check do?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comment below. I'm not sure how it does its magic exactly anymore, I just know it reliably yields the desired results.
Exemplary test cases as mentioned below:
Let me know if you feel comfortable with the sort function so we can move forward. Halting any further effort until then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that's behavior that I don't understand: In testcase two,
loc
is the last DC in the input and ends up as first element.In the third test case,
loc
is the first DC, and still ends up as last. Is that supposed to be?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. In the first and second test case, the user input the subject with
CN
being the first element – this is a classic case of assuming the hierarchy goes from left-to-right withCN
being the lowest weighted element.In test case three, we're looking at the opposite. The user's hierarchical input is correct as per
X509
withCN
being the last element.In all three test cases the user respect the general hierarchical-structure, albeit in a reversed manner.