Skip to content

refactor(auth): initialize new logout lib #14970

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

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Conversation

addisonbeck
Copy link
Contributor

@addisonbeck addisonbeck commented May 27, 2025

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-21910

📔 Objective

In order to remove references in @bitwarden/common to @bitwarden/auth I have created a new feature lib, @bitwarden/logout, and moved the LogoutReason type to it.

This removes 11 out of 20 references to @bitwarden/auth from @bitwarden/common and removes some references from @bitwarden/key-management as a bonus.

Taking things one step further: since the new library has no circular dependencies it can be set up to build with Nx. This involved adding @nx/js as a dependency so we can use its tsc executor for building the project.

Commits

Guides For Reviews

Key Management

We've just updated some of your import statements here.

Auth

We've moved the LogoutReason type out of @bitwarden/auth and into a new library, @bitwarden/logout. This allows us untangle many of the circular references between @bitwarden/common and @bitwarden/auth, among others. This establishes a feature based libs pattern in the monorepo, which is a step forward towards our goal of having small libs with a clear dependency graph.

This new library is owned by auth and comes already integrated with Nx tooling.

Platform

I've updated some imports, clarified some CODEOWNERS rules, and added to our Nx configuration.

📸 Screenshots

Screenshot 2025-05-27 at 4 15 20 PM

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@addisonbeck addisonbeck changed the base branch from main to rename-tsconfig May 27, 2025 17:28
Copy link
Contributor

github-actions bot commented May 27, 2025

Logo
Checkmarx One – Scan Summary & Details8f139a8c-6289-43c7-bb68-e804993d44ad

New Issues (1)

Checkmarx found the following issues in this Pull Request

Severity Issue Source File / Package Checkmarx Insight
MEDIUM Missing_HSTS_Header /libs/common/src/services/api.service.ts: 238
detailsThe web-application does not define an HSTS header, leaving it vulnerable to attack.
ID: XbsvGs7wOHm8gcG3cU8GM5FUSMg%3D
Attack Vector

Copy link

codecov bot commented May 27, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 36.33%. Comparing base (32e6f44) to head (d9f6265).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
apps/desktop/src/app/app.component.ts 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                 @@
##           rename-tsconfig   #14970      +/-   ##
===================================================
- Coverage            36.33%   36.33%   -0.01%     
===================================================
  Files                 3200     3199       -1     
  Lines                93302    93300       -2     
  Branches             16855    16855              
===================================================
- Hits                 33904    33902       -2     
  Misses               56962    56962              
  Partials              2436     2436              

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

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@addisonbeck addisonbeck changed the title Logout lib refactor(auth): initialize new logout lib May 27, 2025
@addisonbeck addisonbeck force-pushed the logout-lib branch 6 times, most recently from 0c63d01 to 692078d Compare May 27, 2025 20:21
Copy link

Base automatically changed from rename-tsconfig to main June 2, 2025 20:38
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.

1 participant