get_proposal will always return the same proposal regardless of credentials used

[marcus-oscarsson]

Some functionality adding the possibility to test against several proposals was recently added in #1066. However, get_proposal at this line https://github.com/mxcube/mxcubecore/pull/1066/files#diff-49fa365d70805e215a94138cdd0f725d690114c71b0c7cf53e1ab6d10d055e5fR125 of ISPyBClientMockup will always return the same proposal regardless of login used.

Hence logging-in with idtest1 returns the proposal of idtest0, right ? This means that the tests for mxcubeweb will fail. If you agree I will disable this test in mxcubeweb for the moment and then you can have a closer look at returning the right proposal (when you get some time)

[fabcor-maxiv]

I think I was the one to start this mess here: https://github.com/mxcube/mxcubeweb/pull/1036/files#diff-948603f00beb75aff4ef52d02fed569c1702af735c26ee1fe13496af0679e7fcR104-R140

These tests were likely not testing what they were supposed to test.

[dominikatrojanowska]

ok, I started this discussion in wrong thread (mxcube/mxcubeweb#1488)

Let's add the second proposal then. Those are my propositions to avoid copy pasting the same stuff:

• method taking user id as an argument and returning relevant dictionary eg.

def __test_proposal(self, user: Optional[str] = "") -> dict:
        proposal = None if not user else {
            "status": {"code": "ok"},
            "Person": {
                "personId": user[-1],
                "laboratoryId": 1,
                "login": None,
                "familyName": "operator on IDTESTeh1",
            },
            "Proposal": {
                "code": "idtest",
                "title": "operator on IDTESTeh1",
                "personId": user[-1],
                "number": "0",
                "proposalId": 1,
                "type": "MX",
            },
            "Session": [
                {
                    "scheduled": 0,
                    "startDate": "2013-06-11 00:00:00",
                    "endDate": "2023-06-12 07:59:59",
                    "beamlineName": self.beamline_name,
                    "timeStamp": "2013-06-11 09:40:36",
                    "comments": "Session created by the BCM",
                    "sessionId": 34591,
                    "proposalId": 1,
                    "nbShifts": 3,
                }
            ],
            "Laboratory": {"laboratoryId": 1, "name": "TEST eh1"},
        }
        return proposal

but the reading way would change to self.__test_proposal(user_id)

• or property, overwritten when the user logs in. Reading way wouldn't change (self.__test_proposal)

    @property
    def __test_proposal(self) -> dict:
        return self._test_proposal

    @__test_proposal.setter
    def __test_proposal(self, user: str):
        self._test_proposal = {
            "status": {"code": "ok"},
            "Person": {
                "personId": user[-1],
                "laboratoryId": 1,
                "login": None,
                "familyName": "operator on IDTESTeh1",
            },
        ...

I don't know if anything else should be changed... in my opinion I would keep it as simple and less effort as possible since those are tests only. Tell me if I'm wrong or you have different opinion. Also some explanation of what should differ between those two proposals (corresponding to idtest0 an idtest1) would be appreciated :)

[fabcor-maxiv]

Would these suggestions also allow testing the cases where two users have the same proposal?

[fabcor-maxiv]

some explanation of what should differ between those two proposals (corresponding to idtest0 an idtest1) would be appreciated

I am not super familiar with the topic.

What does two different proposals mean? I guess it means at least different proposalId values (and also different code as well maybe).

I guess we need to be able to test:

• two users same proposal
• two users different proposals

[fabcor-maxiv]

@dominikatrojanowska

I have a change like this in mind (untested):

diff --git a/mxcubecore/HardwareObjects/mockup/ISPyBClientMockup.py b/mxcubecore/HardwareObjects/mockup/ISPyBClientMockup.py
index 8724a1b42..9a5ef1933 100644
--- a/mxcubecore/HardwareObjects/mockup/ISPyBClientMockup.py
+++ b/mxcubecore/HardwareObjects/mockup/ISPyBClientMockup.py
@@ -50,37 +50,60 @@ class ISPyBClientMockup(HardwareObject):
         except AttributeError:
             pass
 
-        self.__test_proposal = {
-            "status": {"code": "ok"},
-            "Person": {
-                "personId": 1,
-                "laboratoryId": 1,
-                "login": None,
-                "familyName": "operator on IDTESTeh1",
-            },
-            "Proposal": {
-                "code": "idtest",
-                "title": "operator on IDTESTeh1",
-                "personId": 1,
-                "number": "0",
-                "proposalId": 1,
-                "type": "MX",
-            },
-            "Session": [
-                {
-                    "scheduled": 0,
-                    "startDate": "2013-06-11 00:00:00",
-                    "endDate": "2023-06-12 07:59:59",
-                    "beamlineName": self.beamline_name,
-                    "timeStamp": "2013-06-11 09:40:36",
-                    "comments": "Session created by the BCM",
-                    "sessionId": 34591,
-                    "proposalId": 1,
-                    "nbShifts": 3,
-                }
-            ],
-            "Laboratory": {"laboratoryId": 1, "name": "TEST eh1"},
-        }
+        self.__test_proposals = self._init_proposals()
+
+    def _init_proposals(self):
+        print("*** *** *** _init_proposals")
+
+        def _build_proposal(proposal_id: int, session_id: int):
+            proposal_code = f"proposal{proposal_id}"
+            proposal_number = "{proposal_id-1}"
+            proposal = {
+                "status": {"code": "ok"},
+                "Person": {
+                    "personId": 1,
+                    "laboratoryId": 1,
+                    "login": None,
+                    "familyName": "operator on IDTESTeh1",
+                },
+                "Proposal": {
+                    "code": proposal_code,
+                    "title": "operator on IDTESTeh1",
+                    "personId": 1,
+                    "number": proposal_number,
+                    "proposalId": proposal_id,
+                    "type": "MX",
+                },
+                "Session": [
+                    {
+                        "scheduled": 0,
+                        "startDate": "2013-06-11 00:00:00",
+                        "endDate": "2023-06-12 07:59:59",
+                        "beamlineName": self.beamline_name,
+                        "timeStamp": "2013-06-11 09:40:36",
+                        "comments": "Session created by the BCM",
+                        "sessionId": session_id,
+                        "proposalId": proposal_id,
+                        "nbShifts": 3,
+                    }
+                ],
+                "Laboratory": {"laboratoryId": 1, "name": "TEST eh1"},
+            }
+            return proposal_code, proposal_number, proposal
+
+        proposals = {}
+
+        (proposal_code, proposal_number, proposal) = _build_proposal(
+            proposal_id=1, session_id=34591
+        )
+        proposals[(proposal_code, proposal_number)] = proposal
+
+        (proposal_code, proposal_number, proposal) = _build_proposal(
+            proposal_id=2, session_id=34592
+        )
+        proposals[(proposal_code, proposal_number)] = proposal
+
+        return proposals
 
     @property
     def loginType(self):
@@ -94,6 +117,8 @@ class ISPyBClientMockup(HardwareObject):
         return self.loginType
 
     def login(self, login_id, psd, ldap_connection=None, create_session=True):
+        print(f"*** *** *** login {login_id=} {psd=}")
+
         # to simulate wrong loginID
         if login_id not in ("idtest0", "idtest1"):
             return {
@@ -230,10 +255,21 @@ class ISPyBClientMockup(HardwareObject):
         :returns: The dict (Proposal, Person, Laboratory, Sessions, Status).
         :rtype: dict
         """
-        return self.__test_proposal
+        return self.__test_proposals[(proposal_code, proposal_number)]
 
     def get_proposals_by_user(self, user_name):
-        return [self.__test_proposal]
+        if user_name in ("idtest0", "idtest1"):
+            proposal_id = 1
+        else:
+            proposal_id = 2
+
+        proposal_code = f"proposal{proposal_id}"
+        proposal_number = f"{proposal_id-1}"
+        proposal = self.__test_proposals[(proposal_code, proposal_number)]
+
+        print(f"get_proposals_by_user({user_name=}) => {proposal=}")
+
+        return [proposal]
 
     def get_session_local_contact(self, session_id):
         return {

[dominikatrojanowska]

Would these suggestions also allow testing the cases where two users have the same proposal?

In my proposition not really... the proposal numbers were dependent on user id

[fabcor-maxiv]

@marcus-oscarsson We realized that we probably do not know enough about proposal-based authentication to design something we are comfortable with. And with the whole authentication topic in general, we are not 100% sure what the behaviour should be in the details and it turns out that we are kind of guessing.

I feel like what should happen is that we all together write down what the behaviour of authentication should be for all known use cases (user-based and proposal-based, multiple users from same proposal and from different proposals, and so on). Does that make sense?

[marcus-oscarsson]

I agree that it would be good to document the process, especially if its not already well understood. I guess the timing of this is a little bit unfortunate or fortunate depending on how one sees it, I believe the later ;). As you know we have also started working on AbstractLims and SSO. We've added some brief documentation on the subject:

https://mxcubecore--1079.org.readthedocs.build/en/1079/dev/lims_integration.html

The PR's can be found here: #1079, mxcube/mxcubeweb#1492

I would suggest that you check this out, and try the user based login. Perhaps it solves the issue your trying to address in: mxcube/mxcubeweb#1455