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

Refactor Realm and related code #24630

Merged
merged 5 commits into from
Oct 12, 2023
Merged

Conversation

arjantijms
Copy link
Contributor

  • Split out the over complicated Realm class is a few pieces that are easier to understand
  • Specifically, create an interface for getting user data (used by the authentication code) and an interface for managing user data (used by admin console)
  • Split between the all static methods and the stateful instance methods
  • Remove classes not longer in use and/or deprecated
  • Move all realm related exceptions to their own package

Note:

There is more to be done still, especially the individual realm implementations can be refactored wrt enhanced for loops, pairing with their LoginModule peer, etc.

* Split out the over complicated Realm class is a few pieces that are
easier to understand
* Specifically, create an interface for getting user data (used by the
authentication code) and an interface for managing user data (used by
admin console)
* Split between the all static methods and the stateful instance methods
* Remove classes not longer in use and/or deprecated
* Move all realm related exceptions to their own package

Note:

There is more to be done still, especially the individual realm
implementations can be refactored wrt enhanced for loops, pairing with
their LoginModule peer, etc.

Signed-off-by: Arjan Tijms <arjan.tijms@omnifish.ee>
Signed-off-by: Arjan Tijms <arjan.tijms@omnifish.ee>
Signed-off-by: Arjan Tijms <arjan.tijms@omnifish.ee>
Signed-off-by: Arjan Tijms <arjan.tijms@omnifish.ee>
Signed-off-by: Arjan Tijms <arjan.tijms@omnifish.ee>
@dmatej dmatej merged commit c152515 into eclipse-ee4j:master Oct 12, 2023
2 checks passed
@dmatej
Copy link
Contributor

dmatej commented Oct 12, 2023

Just one note - shouldn't we release 7.1.0 at the end of the month? This seems like a breaking change, I am not sure, I did not write a realm and login module for a while, but some classes changed packages (exceptions, ie).

@arjantijms
Copy link
Contributor Author

It does depend on what people would use from GlassFish internals. I personally think that if internal code changes (with the exception of the classes listed in the xml files) it would not constitute a breaking change in the traditional sense.

Of course the problem is that GlassFish for building LoginModules/Realms doesn't distinguish between external and internal code. Types that people can use for custom LoginModules/Realms should have gone into one of the many GlassFish API modules, but obviously this has never been done.

@arjantijms arjantijms deleted the refactor_realm branch October 12, 2023 16:04
@dmatej
Copy link
Contributor

dmatej commented Oct 12, 2023

This is rather an API of GlassFish ... years ago I added the security-ee in my dependencies, with provided scope, implemented realm and login module, and then I had a script to install it into the GlassFish (copy the jar file, update server.policy and login.conf).
When you upgrade from 7.0.9 to 7.0.10 (7.1.0), you have to rebuild your library. Sad we don't have any tests for this, perhaps we should write them. Until that this is rather my hypothesis, but I would expect library compiled against 7.0.9 would not work with current 7.0.10-SNAPSHOT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants