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

Add authentication (username and password) to Aspire.Hosting.Seq containers #6878

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

Conversation

nblumhardt
Copy link

@nblumhardt nblumhardt commented Dec 5, 2024

Description

Opening a draft for feedback/direction - ping @eerhardt 👋

I loosely followed the corresponding PR for this feature in the MongoDB integration, initially adding optional username and password arguments to AddSeq().

I felt a bit uneasy about the difference in semantics between these and the similar arguments in AddMongoDB(): in that case, omitting the arguments causes default values to be used, including for the password, so the result is still secured. In the Seq case, because the username and password are UI features (not provided to other resources) there seems to be no way to surface generated default values, so when the parameters are omitted authentication is not enabled at all.

To signal the difference in semantics, and to make room for some more detailed XDOCs on behavior, I sketched an IResourceBuilder<SeqResource>.WithAuthentication() extension method instead. Not sure if this aligns with how the API is designed or meets discoverability goals, so let me know if parameters on AddSeq() would be preferred instead.

I'm also uncertain if/where to add tests for this, so any pointers in that direction would also be appreciated. Cheers! :-)

See #6155,

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
Microsoft Reviewers: Open in CodeFlow

@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Dec 5, 2024
@eerhardt
Copy link
Member

My original intention/thought is to use password based auth by default, like the other integrations - MongoDB, Postgres, RabbitMQ, etc.

In the Seq case, because the username and password are UI features (not provided to other resources) there seems to be no way to surface generated default values, so when the parameters are omitted authentication is not enabled at all.

Can you explain this a bit more?

The rough design is for the Hosting integration to generate a password (which gets persisted to User Secrets in between runs), and then it tells both the docker container and the .NET Web Application that password.

The docker container starts up and only accepts connections using that password. The .NET Web Application uses that password to connect to the container.

Is that not possible with Seq?

@nblumhardt
Copy link
Author

This could be my lack of familiarity with Aspire showing. How would the user obtain one of these generated/default passwords in order to log into the Seq UI?

@eerhardt
Copy link
Member

This could be my lack of familiarity with Aspire showing. How would the user obtain one of these generated/default passwords in order to log into the Seq UI?

(This is my lack of Seq knowledge showing) Is the same password used between a user logging into the UI and an app sending data to Seq? Or is it possible to have separate passwords? No password for the UI and a password based connection string in the app sending data.

because the username and password are UI features

This makes it sound like there is no password/secret that the app sends. Does the app need an access key or something?

@nblumhardt
Copy link
Author

Is the same password used between a user logging into the UI and an app sending data to Seq? Or is it possible to have separate passwords? No password for the UI and a password based connection string in the app sending data.

Apps use API keys when sending data; these are usually given only Ingest permission, and not full read/write/setup access.

Each app or service should be assigned a unique API key so that ingestion can be tracked/managed independently for different sources. Because of this there's not usually a single/fixed API key provisioned when a new Seq instance is configured - API keys have to be generated through the UI, or created by calling the container's HTTP API when authenticated as a user.

Users log in with usernames/passwords or SSO credentials; users generally end up with much broader read/write/setup permissions.

Ideally, both user access and ingestion would be secured. It's possible to secure the UI without securing ingestion, but not vice-versa. In terms overall security benefit, the UI is definitely the more important of the two.

I'm open to trying anything - perhaps the service-level "AddSeq" (as opposed to the hosting one) could try creating API keys on start-up, if user credentials are available? (In production this probably wouldn't be a realistic approach - each service would be assigned a pre-created API key - but IIRC we're not really targeting production deployment with the Seq hosting integration.)

Keen to know what you think. Thanks for taking a look!

@danmoseley
Copy link
Member

@eerhardt could you respond to answer given above

@nblumhardt nblumhardt mentioned this pull request Feb 14, 2025
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants