Created AbstractAuthenticator and made necessary updates to LdapLogin #822
Conversation
Coverage Report •
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
marcus-oscarsson
changed the title
|
@rhfogh, @MartinSavko: Do you see an issue with this on the Qt client side ? |
|
I am a little confused. I do not see AbstractAuthenticator or the relevant mockup anywhere in either code or config files, in the PR or the pre-existing code. Should they not be there? |
marcus-oscarsson
force-pushed
the
mo-abstract-authenticator
branch
from
f719c18 to
4f29862
Compare
|
@rhfogh :) thanks, I forgot to add those files, done now |
| def get_field_values(self): | ||
| return self._field_values | ||
|
|
||
| def invlidate(self): |
| super().init() | ||
|
|
||
| @abc.abstractmethod | ||
| def authenticte(self, username: str, password: str) -> bool: |
There was a problem hiding this comment.
Typo: authenticate
This one is still spelled wrong
There was a problem hiding this comment.
Ohh my, I'm need to go the eye doctor :) thanks !
marcus-oscarsson
force-pushed
the
mo-abstract-authenticator
branch
from
9f3965f to
b61fdf0
Compare
|
Does this pull request imply some changes that a facility should make in their facility-specific code? Or is that intended to be "backwards compatible" so to say? And this pull request from the web part mxcube/mxcubeweb#1140 is required, is that right? Or are the 2 pull requests independent? |
|
This PR and mxcube/mxcubeweb#1140 are independent. It makes it possible to decouple the authentication bit from the ISPyB client (for instance in your case ISPyBrestClient) from the rest. EDIT The actual decoupling of ISPyB and authentication is for a future PR |
| ("Using Authenticatior from ISPyBClient is depricated, | ||
| "use Authenticator to authenticate spereatly and then login to ISPyB"), |
There was a problem hiding this comment.
This is not valid syntax, is it? Maybe missing a closing " on line 744.
I am confused, if I am right and it is not valid syntax, then shouldn't it immediately fail? :D
(Also typo: "deprecated" not "depricated".)
There was a problem hiding this comment.
Indeed - this is invalid - my editor says so too. Maybe ISPyBClient is not imported?
There was a problem hiding this comment.
Yes, it was missing a "
Greetings from the MXCuBE meeting :)
Introduced
AbstractAuthenticatorand made necessary updates toLdapLoginThis also means that we should move the
authenticatecall fromISPyBclientto the the authenticate/loginlogic.
All
*LdapLoginclasses have been updated so that they care now call*Authenticator