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.

Sign up for GitHub

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

Jump to bottom

working with sessions #1385

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

working with sessions #1385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d2a4464
do not create session for all users proposals
meguiraun Aug 26, 2024
7df3ad8
Update mxcubeweb/core/components/user/usermanager.py
meguiraun Sep 2, 2024
a8e938a
Update mxcubeweb/core/components/user/usermanager.py
meguiraun Sep 2, 2024
e5efd4e
Update mxcubeweb/core/components/user/usermanager.py
marcus-oscarsson Sep 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions mxcubeweb/core/components/lims.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,6 @@ def lims_login(self, loginID, password, create_session):
)
return ERROR_CODE

for prop in session["proposal_list"]:
todays_session = HWR.beamline.lims.get_todays_session(prop)
prop["Session"] = [todays_session["session"]]

if hasattr(
HWR.beamline.session, "commissioning_fake_proposal"
) and HWR.beamline.session.is_inhouse(loginID, None):
Expand Down Expand Up @@ -326,7 +322,7 @@ def select_proposal(self, proposal):
)

todays_session = HWR.beamline.lims.get_todays_session(
proposal_info, create_session=False
proposal_info, create_session=True
Copy link
Member

@marcus-oscarsson marcus-oscarsson Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Hm, the sessions should already exist when its being selected, at least with the current logic. select_proposal will for instance be called when the application is fetched so creating a session here would break that logic. A session otherwise created above:

https://github.com/mxcube/mxcubeweb/pull/1385/files#diff-3c54a0ef3e4a4efd2c578368ac1933aa853c8f8553112067cc4c75058a25b476R242

The problem is maybe in your case that you also call this logic when a user selects a proposal when login-in as user ? In that case, would it be possible to add an argument create_proposal from that route. I imagine its post to /porposal ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

get_todays_session only creates a session if it does not exist. So in my view, having a True here is safe in any situation (at this point the user has made a decision, the proposal to be used).

the https://github.com/mxcube/mxcubeweb/blob/develop/mxcubeweb/core/components/user/usermanager.py#L331 has the flag False always, so in our case the session is not going to be created, plus it would have been created for potentially a proposal that is not the one to be selected just after

sure we can add an argument to the call, but I still dont get why...

Are you using user account based login? Maybe easier if we discuss this during tomorrow's meeting

Copy link
Member

Choose a reason for hiding this comment

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

We are starting to use the user based logins and we are currently evaluating the SSO integration. There will soon be PR with our draft implementation, and we will see exactly how to continue from there.

Regarding the above, Its not entirely clear to me and it might need some testing to verify. I think one issue is that the call to get_proposal_info, above does not check with LIMS for created proposals after login,

mxcubeweb/mxcubeweb/core/components/lims.py

Line 304 in a8e938a

proposal_info = self.get_proposal_info(proposal)

So that proposal_info is not updated and a new session is created.

Meaning that, I guess, any call to get_todays_session, would create a new session. The session I guess would be created on the LIMS side but would not be seen by mxcube until a new login is made. I think that is why the flag is false.

The logic here is a bit fragile and we've not reworked this as we are also working on the SSO integration.

I believe you if you say that youv'e tested and that it works

)
HWR.beamline.session.session_id = todays_session.get("session").get(
"sessionId"
Expand Down
10 changes: 6 additions & 4 deletions mxcubeweb/core/components/user/usermanager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,10 +383,12 @@ def _login(self, login_id: str, password: str):
raise Exception("Remote access disabled")

# Only allow remote logins with existing sessions
if self.app.lims.lims_valid_login(login_res) and is_local_host():
if not self.app.lims.lims_existing_session(login_res):
Copy link
Member

Choose a reason for hiding this comment

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

This would break the idea of that a session needs to exist to be able to login with proposal based logins. We need to check the login type in that case

login_res = self.app.lims.create_lims_session(login_res)

if (
self.app.lims.lims_valid_login(login_res)
and is_local_host()
and HWR.beamline.lims.loginType.lower() != "user"
):
and HWR.beamline.lims.loginType.lower() != "user":
Copy link
Member

@marcus-oscarsson marcus-oscarsson Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Suggested change
and HWR.beamline.lims.loginType.lower() != "user":
if not self.app.lims.lims_existing_session(login_res):
login_res = self.app.lims.create_lims_session(login_res)

Copy link
Member

Choose a reason for hiding this comment

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

Ok, final attempt, with these suggestions for now, You get the idea :)

msg = "[LOGIN] Valid login from local host (%s)" % str(info)
logging.getLogger("MX3.HWR").info(msg)
elif self.app.lims.lims_valid_login(
Expand Down
Loading