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

The Authentication class is now final #86544

Merged
merged 2 commits into from
May 17, 2022

Conversation

ywangd
Copy link
Member

@ywangd ywangd commented May 9, 2022

This PR marks the Authentication class to be final which is the last
step of locking down the Authentication class. The Authentication class
is basically a record class and it has internal logics on what values
can or cannot be used together. We don't expect it to be extended by any
subclasses and a concrete object should always be created for tests
instead of mocking.

Relates: #86424
Relates: #86206

This PR marks the Authentication class to be final which is the last
step of locking down the Authentication class. The Authentication class
is basically a record class and it has internal logics on what values
can or cannot be used together. We don't expect it to be extended by any
subclasses and a concrete object should always be created for tests
instead of mocking.

Relates: elastic#86424
Relates: elastic#86206
@ywangd ywangd added >refactoring :Security/Security Security issues without another label v8.3.0 labels May 9, 2022
@ywangd ywangd requested a review from albertzaharovits May 9, 2022 01:16
@elasticmachine elasticmachine added the Team:Security Meta label for security team label May 9, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (Team:Security)

Copy link
Contributor

@albertzaharovits albertzaharovits left a comment

Choose a reason for hiding this comment

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

LGTM

Well done pushing this through 🎊

@ywangd
Copy link
Member Author

ywangd commented May 17, 2022

@elasticmachine update branch

@ywangd ywangd merged commit ac7fc11 into elastic:master May 17, 2022
ywangd added a commit to ywangd/elasticsearch that referenced this pull request Jul 13, 2022
This PR is a follow-up of elastic#86246 to further clean up the Authentication
class by:
* Promoting usage of the Subject class. The User field and a few other
  related fields are now removed from Authentication. Relevant methods
  have their implementation replaced by using Subjects and same
  behaviours are retained.
* Removed the temporary internal RunAsUser class. It essence is about
  wire serialisation which is now merged into Authentication itself.
* Simplify serialisation of regular User object. All the complexities of
  handling inner user is now completely within the Authentication class
  itself.
* Cosnolidate assertions in different places into a single method that
  is called in constructors. Also removed a few assertions because there
  is no RunAsUser class anymore and a User object is just a simple user.

Relates: elastic#86246
Relates: elastic#86544
elasticsearchmachine pushed a commit that referenced this pull request Jul 14, 2022
This PR is a follow-up of #86246 to further clean up the Authentication
class by: * Promote usage of the Subject class. The User field and a few
other   related fields are now removed from Authentication. Relevant
methods   have their implementation replaced by using Subjects and same 
behaviours are retained. * Remove the temporary internal RunAsUser
class. Its essence is about   wire serialisation which is now merged
into Authentication itself. * Simplify serialisation of regular User
object. All the complexities of   handling inner user is now completely
within the Authentication class   itself. * Consolidate assertions in
different places into a single method that   is called in constructors.
Also removed a few assertions because there   is no RunAsUser class
anymore and a User object is just a simple user.

Relates: #86246 Relates: #86544

Resolves: #80117
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>refactoring :Security/Security Security issues without another label Team:Security Meta label for security team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants