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 custom authenticator to be added via configuration #115

Merged
merged 6 commits into from Sep 27, 2013

Conversation

Projects
None yet
2 participants
@georgy
Contributor

georgy commented Sep 15, 2013

Introduce new configuration parameter (authenticatorClass) that is used to specify custom authenticator class. This authenticator class will be found, instance will be created (if class has a constructor that takes Configuration as parameter, jolokia runtime configuration will be provided, allowing authenticator to take it into account). Newly created instance of authenticator will be set for Jolokia server to use. If authenticatorClass config parameter was not provided, code will fall back to old logic (i.e. check if user/password is set and create default authenticator).

This pull request also adds a ugly looking code into copyResourceToTemp method of JvmAgentConfigTest with the main goal of properly escaping paths used in tests when they are run win (tested on win7).

@georgy

This comment has been minimized.

Contributor

georgy commented Sep 23, 2013

Any chance this can make next minor release :)? How can I help this to get integrated?

@rhuss

This comment has been minimized.

Owner

rhuss commented Sep 24, 2013

Looks fine, thanks a lot ! I'll apply the pull request ASAP, assuming that APL as license is ok for you.

BTW, in the initial description it is mentioned, that a customer authenticator gets a Configuration object as argument for its constructor, but as far as I see, your code uses only the default constructor. That's no problem for me, just wanted to mention it.

@rhuss

This comment has been minimized.

Owner

rhuss commented Sep 24, 2013

BTW, sorry for the delay, I'm super busy nowadays. But I will push out a 1.1.4-SNAPSHOT this week with this change applied.

@georgy

This comment has been minimized.

Contributor

georgy commented Sep 24, 2013

No worries, just checking in :).

Re Configuration: Implementation will try two constructors, first it will try to find constructor that takes Configuration, if there is none it will fall back to default (no-arg) constructor.

@rhuss

This comment has been minimized.

Owner

rhuss commented Sep 24, 2013

Ah, ok. Overlooked this.

Could you do me a favor and add this configuration parameter to src/docbkx/agents/jvm.xml and add this as a new feature to src/changes/changes.xml 'would speed up things, but I can do it, too later on.

@georgy

This comment has been minimized.

Contributor

georgy commented Sep 24, 2013

Sure thing, I will add this in couple of hours

@georgy

This comment has been minimized.

Contributor

georgy commented Sep 25, 2013

Done

@rhuss

This comment has been minimized.

Owner

rhuss commented Sep 26, 2013

Thanks a lot, I will apply the pull request ASAP. You can expect a 1.1.4 release this weekend probably, worst case is end of next week.

@georgy

This comment has been minimized.

Contributor

georgy commented Sep 26, 2013

Cool, thanks

rhuss added a commit that referenced this pull request Sep 27, 2013

Merge pull request #115 from georgy/master
Allow custom authenticator to be added via configuration

@rhuss rhuss merged commit 9218aac into rhuss:master Sep 27, 2013

1 check passed

default The Travis CI build passed
Details
@rhuss

This comment has been minimized.

Owner

rhuss commented Sep 27, 2013

Thanks again. Hopefully this weekend I've more time in order to prepare a release.

@rhuss

This comment has been minimized.

Owner

rhuss commented May 28, 2015

FYI, I'm about to release 1.3.1 today and for consistencies sake I renamed 'authenticationClass' to 'authClass' as parameter. I hope this does not provide you any issues when doing upgrades. If this is a stumbling block, though, I can introduce this parameter back as a fallback (but would like to avoid this to keep the code base clean).

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