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.

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

maint(authd): Expose public API to build and parse UI layouts #788

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

3v1n0
Copy link
Collaborator

@3v1n0 3v1n0 commented Feb 11, 2025

This is the continuation of #626 that allows to have a single codebase to both parse and generate UI layouts from the client and broker sides.

Compared to the initial version of this change, now the types that we export are still based on the proto definitions, but we generate them with a new code generator that ensures that types can be exported and that does 1:1 copies when required.

Broker side: ubuntu/authd-oidc-brokers#234

UDENG-5522

@3v1n0 3v1n0 requested a review from a team as a code owner February 11, 2025 04:03
Comment on lines +38 to +44
case UIQrCode:
return QrCode
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't rename these on purpose (I didn't want to change behavior), but I think it's now a good moment for doing it... So likely this should be now called externally WebAuth or something similar (I wouldn't call it device auth since that's not dependent on any device per se, but rather on a web authentication triggered through the content URI).

@codecov-commenter
Copy link

codecov-commenter commented Feb 11, 2025

Codecov Report

Attention: Patch coverage is 90.63232% with 40 lines in your changes missing coverage. Please review.

Project coverage is 83.13%. Comparing base (36511cd) to head (ee0d2d8).
Report is 366 commits behind head on main.

Files with missing lines Patch % Lines
internal/examplebroker/broker.go 75.00% 12 Missing and 7 partials ⚠️
internal/services/pam/pam.go 66.66% 4 Missing and 2 partials ⚠️
internal/proto/authd/authmode.go 50.00% 5 Missing ⚠️
brokers/auth/authmode.go 0.00% 4 Missing ⚠️
brokers/auth/mode.go 93.33% 2 Missing and 1 partial ⚠️
brokers/layouts/uilayout.go 91.89% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #788      +/-   ##
==========================================
- Coverage   83.43%   83.13%   -0.31%     
==========================================
  Files          83      101      +18     
  Lines        8689     9880    +1191     
  Branches       74       74              
==========================================
+ Hits         7250     8214     +964     
- Misses       1111     1277     +166     
- Partials      328      389      +61     

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

3v1n0 added 14 commits February 11, 2025 16:05
We may want to expose some proto types externally, but these needs to be
sanitized so that they don't include any type we may not want to expose.

At the same time we want to have automatic conversion without having to
deal maintaining generated code... So here it is the types-parser:
 - It introspects types given the parameters provided
 - Generates a list of types
 - Creates functions wrappers and conversion functions
…se them

We use to create UI options manually and converting them to maps for
being transferred via DBus, it's something we also need to do in the
broker side, so let's expose this publicly for being reused by brokers
too
If some values are unset, there's no point to expose them to the brokers
Now that it has been tested with the manually generated version we can
use the utility functions.
It can be useful for brokers to handle the received messages
…layouts

It adds a convenient way to handle the value as a native type.
These can be used by brokers and authd to parse the results back and
forth from the dbus format.
Now that authd can be imported we should not expose anything that isn't
an API.
@3v1n0 3v1n0 force-pushed the authd-brokers-api branch from 20d567f to ee0d2d8 Compare February 11, 2025 15:08
@adombeck adombeck self-requested a review February 11, 2025 15:21
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.

2 participants