Skip to content

Remove all code related to the x509_presented_hash_attribute_requested_issuers#10701

Merged
Sgtpluck merged 2 commits intomainfrom
dmm/remove-x509-gate
May 29, 2024
Merged

Remove all code related to the x509_presented_hash_attribute_requested_issuers#10701
Sgtpluck merged 2 commits intomainfrom
dmm/remove-x509-gate

Conversation

@Sgtpluck
Copy link
Copy Markdown
Contributor

🎫 Ticket

https://gitlab.login.gov/lg-people/lg-people-appdev/protocols/backlog-fy24/-/issues/16

🛠 Summary of changes

This change is removing vestigial code left over from fixing the x509:presented attribute bug. The changes include:

  • removing the backwards compatibility conditional
  • removing the configuration key from the IdentityConfig and application.yml.default
  • removing the now unnecessary test

@Sgtpluck Sgtpluck requested a review from a team May 28, 2024 14:44
@@ -173,13 +173,7 @@ def x509_session?
end

def x509_presented
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

since we're here, and this definitely returns a boolean now, WDYT about renaming to have a ? (also good for a follow-up PR if we want to keep this one small)

Suggested change
def x509_presented
def x509_presented?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh yeah actually ... i could just remove this method now and inline it since it's so smol!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

moved in 27e5a37

@Sgtpluck Sgtpluck merged commit a93e7dc into main May 29, 2024
@Sgtpluck Sgtpluck deleted the dmm/remove-x509-gate branch May 29, 2024 17:40
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.

2 participants