@andydunstall Thank you for the quick turnaround here!

I gave this a whirl today, and here are my thoughts:

1. The --connect.tenant-id validation and authentication works as expected, when we're trying to register an upstream with a known tenant id on the server, and rejects the connection when given an unknown tenant id
2. The lack of validation for the endpoint name still allows agents to potentially register with whatever name they desire, which seems to conflict with the purpose of the tenant ID in the first place
3. There doesn't seem to be any validation of the agents trying to register when --connect.tenant-id is omitted -- I would have expected that the upstream server would reject any connections that do not specify a tenant-id to use, when we have upstreams.tenants defined

ok thanks for trying it out

Thank you for the quick turnaround here!

I'm not sure I'd call 6 weeks quick... 😁

The lack of validation for the endpoint name still allows agents to potentially register with whatever name they desire, which seems to conflict with the purpose of the tenant ID in the first place

Sure. I thought about adding that, though you can already encode the permitted endpoint IDs in the JWT token, so I didn't see much point adding the same thing again in the tenant configuration

Would encoding the endpoint ID in the token work for you?

I would have expected that the upstream server would reject any connections that do not specify a tenant-id to use, when we have upstreams.tenants defined

Though in that case the tenant would be incorrectly authenticated? Just as if a tenant attempts to authenticate as another tenant, they'd get rejected, the same applies if they attempt to connect without a tenant ID? (i.e connect to the 'default' tenant)

Sure. I thought about adding that, though you can already encode the permitted endpoint IDs in the JWT token, so I didn't see much point adding the same thing again in the tenant configuration

Yeah, that's fair. I'm approaching this from a perspective a single service proxied per-tenant, but I suppose it is a great option to allow multiple services to be proxied per-tenant, so I think the JWT claims encoding would make sense.

Though in that case the tenant would be incorrectly authenticated? Just as if a tenant attempts to authenticate as another tenant, they'd get rejected, the same applies if they attempt to connect without a tenant ID? (i.e connect to the 'default' tenant)

I'm not sure I fully understand here -- in my mind if an upstream has a list of known tenants (defined using upstream.tenants, it would reject anyone trying to connect with an unknown tenant ID, including when there's no tenant ID specified. Let me know if I'm missing something here

@andydunstall

I'm not sure I fully understand here -- in my mind if an upstream has a list of known tenants (defined using upstream.tenants, it would reject anyone trying to connect with an unknown tenant ID, including when there's no tenant ID specified. Let me know if I'm missing something here

But to enable multi-tenancy, the 'default' tenant must be authenticated

So when an agent attempts to connect to the 'default' tenant, they'll be rejected as they are incorrectly authenticated. Just as if they tried to connect to a tenant they aren't authenticated for

In this case, when you refer to the "default tenant", are you referring to the piko upstream instance itself? And not any other external agents? @andydunstall

If that is the case, makes sense to me!

I'm refering to 'default tenant' as when agents connect without specifying an tenant ID

But to enable multi-tenancy, the 'default' tenant must be authenticated

Okay, I think I'm starting to understand what you're saying now -- I suppose we can ensure that we specify distinct keys for each tenant, including the default tenant to solve the case I'm thinking of, where we we cannot allow any connections to the default tenant, as that would lead to an unknown client connecting to the Piko upstream.

What do you think about including a log line when a connection is made to the default tenant when there is a list of known tenant IDs specified in the server config? It would help catch such misconfigurations for us.

I think I agree with the approach you have here

But to enable multi-tenancy, the 'default' tenant must be authenticated

Coming back to this, is there a reason we need to couple the two? i.e. only allow for multi-tenancy if the default tenant has a key defined?

Hey @dipack95, sorry for only just getting back to you on this

I suppose we can ensure that we specify distinct keys for each tenant

Isn't that the whole point of adding multi-tenancy? To ensure each client can only authenticate with it's own tenant

Coming back to this, is there a reason we need to couple the two? i.e. only allow for multi-tenancy if the default tenant has a key defined?

For the reason above? Whats the point of multi-tenancy if clients can always connect to other tenants (including default tenants)?