Skip to content

Add login to blueapi - #70

Draft
noemifrisina wants to merge 18 commits into
mainfrom
64-login-dialog
Draft

noemifrisina wants to merge 18 commits into
mainfrom
64-login-dialog

Conversation

@noemifrisina

Copy link
Copy Markdown
Collaborator

Closes #64

@codecov

codecov Bot commented Jul 24, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.36%. Comparing base (ae802f6) to head (5152b5f).

Files with missing lines Patch % Lines
src/i19serial_ui/gui/widgets/login_dialog.py 80.00% 11 Missing ⚠️
src/i19serial_ui/gui/serial_gui_eh2.py 85.71% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #70      +/-   ##
==========================================
- Coverage   94.06%   93.36%   -0.70%     
==========================================
  Files          26       27       +1     
  Lines        1415     1493      +78     
==========================================
+ Hits         1331     1394      +63     
- Misses         84       99      +15     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ZohebShaikh ZohebShaikh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Apologies for the late review, I had it on my mind on Friday but couldn't get to it.

The PR looks good

Comment thread src/i19serial_ui/gui/widgets/login_dialog.py Outdated
Comment thread src/i19serial_ui/gui/widgets/login_dialog.py Outdated
Comment thread src/i19serial_ui/gui/serial_gui_eh2.py Outdated
if self.login_dialog.isVisible():
self.login_dialog.close()
tidy_up_logging([self.gui_logger])
print("PLEASE LOG OUT FROM BLUEAPI")

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The cache is getting stored at ~/.cache/blueapi_cache. It would be nice you could get this stored in memory.
This will be a change in blueapi. You will need to create a new In memory Cache.

Then once the UI process terminates the cached tokens are also gone, because my assumption is that people are logging in as a shared user on some i19 beamline computer.

@ZohebShaikh ZohebShaikh Sep 8, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

From our last conversation, I remember that this is not required because everyone logs in to the DLS machine as themselves. And the token is saved in their personal home directory rather than a shared location.

But I'm leaving this to your discretion

@ZohebShaikh ZohebShaikh left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fundamentally this is all good

self.close()
except Exception as e:
log_to_gui(self.logger, "Login failed", level="ERROR")
self.logger.exception(e)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A logout can be added like this

    def _on_click_trigger_logout(self):
        try:
            self.client.client.logout()
            self.user_fedid.emit("")
            self.close()
        except Exception as e:
            log_to_gui(self.logger, "Logout failed", level="ERROR")
            self.logger.exception(e)

Comment on lines +36 to +66
def create_layout(self):
layout = QtWidgets.QVBoxLayout()
lbl = QtWidgets.QLabel("Please log in to run")
lbl.setFont(QtGui.QFont("Arial", 10))
lbl.setAlignment(Qt.AlignmentFlag.AlignCenter)
self.btn = QtWidgets.QPushButton("LOGIN")
self.btn.setMaximumHeight(20)
self.btn.setMaximumWidth(50)
self.btn.clicked.connect(self._on_click_trigger_login)
layout.addWidget(lbl)
layout.addWidget(self.btn)
self.setLayout(layout)

def _load_access_token(self) -> str:
with open(self._token_path, "rb") as fh:
token = base64.b64decode(fh.read()).decode("utf-8")
return token

def _decode_access_token(self) -> dict[str, Any]:
# NOTE Proper way would be to get the jwt_uri from keycloak
# But since blueapi will do the verification and we just need to read the fedid
# this may just be enoug
token = self._load_access_token()
payload = jwt.decode(token, options={"verify_signature": False})
return payload

def _update_user_fedid(self) -> str:
access_token = self._decode_access_token()
if not access_token:
raise ValueError("Could not get access token")
return access_token.get("fedid") # type: ignore

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The code as is will not work

    def _get_session_manager(self) -> SessionManager | None:
        try:
            return SessionManager.from_cache(
                auth_token_path=self.client._blueapi_config.auth_token_path
            )
        except FileNotFoundError:
            # No cached session yet, i.e. user has never logged in
            return None

    def _update_user_fedid(self) -> str | None:
        sm = self._get_session_manager()
        if sm is not None and (access_token := sm.get_valid_access_token()) != "":
            # Logged In
            # Get fedid
            return sm.decode_jwt(access_token).get(USER_ID_CLAIM)

    def create_layout(self):
        layout = QtWidgets.QVBoxLayout()
        self.lbl = QtWidgets.QLabel()
        self.lbl.setFont(QtGui.QFont("Arial", 10))
        self.lbl.setAlignment(Qt.AlignmentFlag.AlignCenter)
        self.auth_btn = QtWidgets.QPushButton()
        self.auth_btn.setMaximumHeight(20)
        layout.addWidget(self.lbl)
        layout.addWidget(self.auth_btn, alignment=Qt.AlignmentFlag.AlignCenter)
        self.setLayout(layout)
        self._refresh_login_state()

    def _refresh_login_state(self):
        if (fedid := self._update_user_fedid()) is None:
            # Not logged in
            self.lbl.setText("Please log in to run")
            self.auth_btn.setText("LOGIN")
            trigger = self._on_click_trigger_login
        else:
            self.lbl.setText(f"Logged in as {fedid}")
            self.auth_btn.setText("LOGOUT")
            trigger = self._on_click_trigger_logout

        try:
            self.auth_btn.clicked.disconnect()
        except TypeError:
            # Not connected to anything yet
            pass
        self.auth_btn.clicked.connect(trigger)

    def showEvent(self, a0: QtGui.QShowEvent| None):
        self._refresh_login_state()
        super().showEvent(a0)

I'm refreshing the state in the showEvent which triggered each time the widget is created.
I'm using the session manager defined in bluaepi to do all the decode of token as you are using the same for login.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The code in it's current state will not work because in the cache I'm not just storing the token.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a log in dialog

2 participants