User

{
    full_name: string,
    username: identifier,
    default_channel: string,
    groups?: identifier[]
}

Group

{
    name: identifier,
    users?: identifier[]
}

So there will be two places where group memberships can be updated? Will the behavior be that on an update, memberships that aren't given, will be removed, i.e. to add a group to a user, you have to query the user first? Does omitting the groups/users attribute mean, that memberships remain unchanged?

• The username column of the contact table must not be nullable

That column is used to store an optional reference to an Icinga Web user. That would imply that each contact is linked to one then.

• The name column of the contactgroup table must be unique

That column currently stores a display name. Sure, that can be encoded as part of an URL, but is this desired here?

But in general, sounds like you want/need a user-chosen primary key for those tables. I wouldn't rule out just doing this instead.

So there will be two places where group memberships can be updated? Will the behavior be that on an update, memberships that aren't given, will be removed, i.e. to add a group to a user, you have to query the user first? Does omitting the groups/users attribute mean, that memberships remain unchanged?

Yes. Yes. Yes. My expectation is that whoever uses this API, just imports data from another source, so there's no need to fetch anything first, as all information is already available.

But in general, sounds like you want/need a user-chosen primary key for those tables. I wouldn't rule out just doing this instead.

I had a discussion with Eric about the use of UUIDs, which I'd chosen first. Though, they'd still need to reference an identifier (UUIDv5) that is known to both sides, i.e. mandatory in any case. Otherwise (UUIDv4) we'd had to prevent changes in the UI to resources created through the API and vice versa. The primary key isn't an option, hence the username. The group name is indeed more of a label right now.

My goal was, not to limit edits in any way. A resource created through the API should be changeable in the UI. This means, by creating one in the UI, the identifier must be provided, just the same as when creating it through the API.

I had a discussion with Eric about the use of UUIDs, which I'd chosen first. Though, they'd still need to reference an identifier (UUIDv5) that is known to both sides, i.e. mandatory in any case. Otherwise (UUIDv4) we'd had to prevent changes in the UI to resources created through the API and vice versa. The primary key isn't an option, hence the username. The group name is indeed more of a label right now.

My goal was, not to limit edits in any way. A resource created through the API should be changeable in the UI. This means, by creating one in the UI, the identifier must be provided, just the same as when creating it through the API.

Sounds like you think something is a problem where I'd say it's perfectly fine. UUIDs are an obvious choice, so let's stick to that. I'd say it's perfectly fine to make the primary key a UUID without any restrictions on the version. If a contact ist created using Web, it just gets a random UUID (v4) assigned. If a contact is created using the API, the client chooses the UUID however it desires. If it syncs from somewhere else that already uses an UUID to identify the source object, use that, otherwise, use a hash-based UUID (v5) or even use a random UUID and keep some state, doesn't matter for us, that would be a decision for the API client author.

Creating a contact in Web and then updating it using the API should be possible, yes, but if your sync source is the primary data source anyways, why wouldn't you create all contacts using the API? If your use-case is to just update individual attributes like an e-mail address for existing contacts, the API client could still query the contact by username using the filter mechanism and then update it by the returned ID. Or it could just query all contacts and then update those, where an update is necessary. So that would still be possible without a predictable ID.

Fine. Let's use UUIDs as identifier. contact.username and contactgroup.name are left untouched then. The UUID of a resource will be part of the structure as id key. But I wouldn't make the UUID the new primary key. We'd have to change every reference in the schema to contacts and groups then.

Oh, btw, I forgot to include addresses. -.-

But I wouldn't make the UUID the new primary key. We'd have to change every reference in the schema to contacts and groups then.

Yes, but it sounds like the way cleaner option, doesn't it? Would this result in an unreasonable amount of work in Notifications Web?

Yes, but it sounds like the way cleaner option, doesn't it?

It's not required. I'd rather add a new column for this, as a start. We still don't have versioning, so the schema isn't stable anyway..

However, if we just replace the primary key type of these two columns, it's a bit of an arbitrary mix.

Another idea for how two different columns could make sense in my opinion: keep the current numeric ID as-is and add a second column, something like external_id or external_key that is nullable and unique. For objects created using the web interface, this value is just not set, but if objects are created via the API, it's an optional field the client can use to store auxiliary information to later identify the same record. I.e. the default identifier stays the numeric ID, but if desired, the API client could access the objects also by this external ID (or username for that matter), all of these would be fast due to the existence of a corresponding index. The type could either be UUID/16 bytes or even just some string type so that the client could store whatever they want, like LDAP DN, ID reference to whatever other database, etc. without requiring any hashing, thus allowing lookups in the other direction as well.

the default identifier stays the numeric ID

Nope. That goes against everything I read. If we keep our numeric ID, it's won't be exposed in the API.

The type could either be...

A single type. I really don't want to support multiple ways if UUID is one of them. It should be the only one.

thus allowing lookups in the other direction as well.

This is something we already solved and agreed on:

Creating a contact in Web and then updating it using the API should be possible, yes, but if your sync source is the primary data source anyways, why wouldn't you create all contacts using the API? If your use-case is to just update individual attributes like an e-mail address for existing contacts, the API client could still query the contact by username using the filter mechanism and then update it by the returned ID.

However, if we just replace the primary key type of these two columns, it's a bit of an arbitrary mix.

Then let's introduce a new column of type UUID and make it required.

the default identifier stays the numeric ID

Nope. That goes against everything I read. If we keep our numeric ID, it's won't be exposed in the API.

Why not? Also, I'm not really sure what you read.

The type could either be...

A single type. I really don't want to support multiple ways if UUID is one of them. It should be the only one.

Of course we should pick one. Just wanted to say that the exact choice won't matter for the rest I wrote.

thus allowing lookups in the other direction as well.

This is something we already solved and agreed on:

That's not what I wanted to say with that. If you have a field large enough to store an unhashed reference, an API client could retrieve a list of all contacts, look them up by say a stored LDAP DN, and check if anything needs to be updated. Not sure how commonly someone would want to build something like this, I just wanted to say that's something that would additionally be possible if it was a "store whatever you want" type.

Why not? Also, I'm not really sure what you read.

To prevent enumeration attacks.

If you have a field large enough to store an unhashed reference, an API client could retrieve a list of all contacts, look them up by say a stored LDAP DN, and check if anything needs to be updated.

If the identifier is a UUID, chosen by the client, no checks are necessary. The client just PUTs the data it has and nothing else. And all with a "store whatever you want" type.

Why not? Also, I'm not really sure what you read.

To prevent enumeration attacks.

Yes, in general, unpredictable IDs add an additional layer of defense. But isn't the API supposed to allow listing all objects anyways?

If the identifier is a UUID, chosen by the client, no checks are necessary. The client just PUTs the data it has and nothing else. And all with a "store whatever you want" type.

Syncing object deletion would be annoying that way though as you don't really have a way of telling how that UUID was generated. If you have an contact that says external_id = 'uid=poorguy,ou=something,dc=example,dc=com', a sync client could simply check if that still exists in LDAP and if it doesn't, issue a DELETE request.

We concluded that two columns (pk + external_uuid), both required, are sufficient for now. Making the pk the UUID can be done at a later point, together with a custom fact for the client to identify its own resources. (To allow safe removals)

Requests

---
payload: invalid

{
    "status": "error",
    "message": "Syntax error: '---\npayload: invalid\n'"
}

---
payload: invalid

{
    "status": "error",
    "message": "No JSON content"
}

{
    "id": "0817d973-398e-41d7-9ef2-61cdb7ef41a2", 
    "users": []
}

{
    "status": "error", 
    "message": "Invalid request body: the fields id and name must be present and of type string"
}

{
    "id": "0817d973-398e-41d7-9ef2-61cdb7ef41a2", 
    "name": "Test group",
    "users": []
}

{
    "status": "error",
    "message": "Filter is only allowed for GET requests"
}

{
    "id": "0817d973-398e-41d7-9ef2-61cdb7ef41a2", 
    "name": "Test group",
    "users": []
}

{
	"status": "error",
	"message": "Contactgroup not found"
}

{
    "id": "0817d973-398e-41d7-9ef2-61cdb7ef41a2", 
    "name": "Test group",
    "users": []
}

{
	"status": "success",
	"data": null
}

{
    "id": "0817d973-398e-41d7-9ef2-61cdb7ef41a2", 
    "name": "Test group",
    "users": []
}

{
	"status": "error",
	"message": "Contactgroup already exists"
}

{
    "id": "0817d973-398e-41d7-9ef2-61cdb7ef41a3", 
    "name": "Test group (replaced)",
    "users": []
}

{
	"status": "success",
	"data": null
}

---
payload: invalid

{
	"status": "error",
	"message": "Syntax error: '---\npayload: invalid\n'"
}

{
    "id": "0817d973-398e-41d7-9ef2-61cdb7ef41a3", 
    "name": "Test group (replaced)",
    "users": []
}

{
	"status": "error",
	"message": "Identifier is required"
}

{
    "id": "0817d973-398e-41d7-9ef2-61cdb7ef41a4", 
    "name": "Test group (replaced)",
    "users": []
}

{
	"status": "error",
	"message": "Identifier mismatch"
}

{
    "id": "0817d973-398e-41d7-9ef2-61cdb7ef41a4",
    "name": "Test group 2",
    "users": []
}

{
	"status": "success",
	"data": null
}

{
    "id": "0817d973-398e-41d7-9ef2-61cdb7ef41a4",
    "name": "Test group 2 (updated)",
    "users": []
}

[
	{
		"id": "0817d973-398e-41d7-9ef2-61cdb7ef41a3",
		"name": "Test group (replaced)",
		"users": []
	},
	{
		"id": "0817d973-398e-41d7-9ef2-61cdb7ef41a4",
		"name": "Test group 2 (updated)",
		"users": []
	}
]

{
	"status": "success",
	"data": [
		{
			"id": "0817d973-398e-41d7-9ef2-61cdb7ef41a3",
			"name": "Test group (replaced)",
			"users": []
		}
	]
}

[
	{
		"id": "0817d973-398e-41d7-9ef2-61cdb7ef41a3",
		"name": "Test group (replaced)",
		"users": []
	}
]

[]

{
	"status": "error",
	"message": "Contactgroup not found"
}

{
	"status": "error",
	"message": "Identifier is required"
}

{
	"status": "error",
	"message": "Contactgroup not found"
}

Broken requests

• Request 1
  This request triggers an uncatched exception as it's passing a YAML, but declares it as application/json.
  The endpoint shouldn't trust HTTP headers but check for the validity of the
  payload before passing it to a JSON decoder.

• Request 5
  The RESTful API should deny this request as it provides a UUID in the Request-URI while using the POST method.
  It currently accepts the request and returns a 404 Not Found as no group with the given UUID could be found.

• Request 9
  This request triggers an uncatched exception as it's passing a YAML, but declares it as application/json.
  The endpoint shouldn't trust HTTP headers but check for the validity of the
  payload before passing it to a JSON decoder.

• Request 16
  There's a few things wrong with this request.
  First of all, the endpoint always returns a list, as there's a filter provided with the request.
  This would be correct, if the filter would get processed as a LIKE (~) condition and not as a full comparison (===, equals).
  It currently does a full match, which is why the result can only contain a single resource when providing an id= filter (since its unique) and multiple resources when providing a name= filter (multiple resources with the same name are technically possible).
  Additionally, there's a problem with the filter parsing mechanism.
  If the request gets executed with brackets in its filter name, the request fails with a 500 Internal Server Error.

  _{Example URI}
  http://localhost/icingaweb2/notifications/api/v1/contactgroups?name=Test%20group%20(replaced)
  _Body

  {
  	  "status": "error",
  	  "message": "Invalid filter \"name=Test group (replaced)\", unexpected ) at pos 26"
  }

Thanks for the tests. Please write directly in the PR next time.
Requests 1 and 9 now throw 400 Bad Request.
Request 5 is implemented as the requirements above. If the ID is specified, an attempt is made to replace the entry.