Skip to content

Conversation

@iinuwa
Copy link
Member

@iinuwa iinuwa commented Aug 13, 2025

Changed

  • Add ARCHITECTURE.md documentation
  • Move D-Bus XML file from contrib to doc

The latter change will change the build scripts.

Closes #84.

@iinuwa iinuwa force-pushed the architecture-docs branch 2 times, most recently from 1049080 to 7ec93ab Compare August 13, 2025 23:09
msirringhaus
msirringhaus previously approved these changes Aug 14, 2025
Copy link
Collaborator

@msirringhaus msirringhaus left a comment

Choose a reason for hiding this comment

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

Left some minor comments, but otherwise looking good!

ARCHITECTURE.md Outdated
the Gateway.

The **UI Control API** is used to launch a UI for the user to respond to
authenticator requests for user interaction. The **Flow Controller** mediates
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even though we have a dedicated API-doc, I feel like the short explanation of the Flow Controller could be a bit more precise.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I'm suffering from the curse of knowledge here. Could you elaborate on how I could expand? I added a couple of sentences, but I don't know if that's sufficient.

ARCHITECTURE.md Outdated

### `credentialsd/src/webauthn.rs`

Types and functions to deal with WebAuthn data.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could be a bit more precise. Something like "Types and functions needed to repackage requests from and responses to JSON-strings according to the WebAuthn spec (link: https://www.w3.org/TR/webauthn-3).
With one notable deviation from the spec: Due to D-Bus limitations, raw binary fields need to be base64 encoded strings.
It is the responsibility of the application using this service to de/construct the field accordingly."

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I was tired by this point 😃.

Thanks for the suggestion. I'll rephrase slightly and use "Due to JSON limitations," since D-Bus is capable of passing binary data, but we're choosing to wrap responses in JSON.

Base automatically changed from readme-update to main August 14, 2025 20:49
@iinuwa iinuwa dismissed msirringhaus’s stale review August 14, 2025 20:49

The base branch was changed.

@iinuwa iinuwa force-pushed the architecture-docs branch from 07d6fa5 to e17d0c9 Compare August 14, 2025 20:53
@iinuwa iinuwa merged commit 469dfff into main Aug 14, 2025
1 check passed
@iinuwa iinuwa deleted the architecture-docs branch August 14, 2025 20:55
@iinuwa
Copy link
Member Author

iinuwa commented Aug 14, 2025

Merged, but if there are any other comments on this guide, we can still discuss here

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 ARCHITECTURE.md

3 participants