Add <server> element into institutions.xml & ro.xml for RadSec

[ghalse]

This patch adds support for v.2.0.1 of the eduroam database schema, in particular adding in the element necessary to allow eduroam CAT to issue RadSec certificates.

The output validates against the schema, as tested by https://monitor.eduroam.org/eduroam-database/v2/scripts/xml_validation_test.php

[ghalse]

Force push to deal with the fact that TLS & DTLS are not enabled by default. The original code had a single if wrapped in a try block to handle both, but assuming both will be enabled is a poor assumption. The revision allows for them to be enabled independently.

[zmousm]

Hi @ghalse, thanks for the patch.

I am still trying to understand the semantics about server_name/server_type, which are not clear to me from the human-readable spec. Give me some time to investigate and I will get back to this.

[ghalse]

So whilst this deals with institutions.xml, it seems we also need this in ro.xml for the NRO to be able to issue certs. That needs some more work.

[zmousm]

Yes, that too. I am not sure however if the semantics for such data are compatible to how such fields are used in djnro. I will look into it.

[ghalse]

Patch b3f89e3 extends the pull request to support ro.xml as well. To avoid overloading the semantics of InstServer, I've extended the model to include RealmServer that's only editable via the admin interface (as is the case with Realm anyway).

Patch 4bdbdf2 amends the initial patch (67b6c07) to make use of the newly-introduced namedtuple with the eduroam DB server type values to make future maintenance easier and more consistent.

[vladimir-mencl-eresearch]

Hi @ghalse ,

Sorry to see this PR left unattended for so long. I've now started looking into RADSEC myself and discovered you've already done all this work.

I've just tried adding your code to our deployment.

Works as it should, and I found only one minor issue with the migration (will post that shortly).

I hope to see this merged soon - will be very useful for the whole community to enable getting RADSEC certificates.

Cheers, Vlad

[vladimir-mencl-eresearch]

Hi @ghalse , @zmousm ,

I've had a further go at getting CAT to pick up TLS servers from my DjNRO export.

With the functionality in this PR, I got eDB+CAT to pick up our NRO servers as TLS servers and I'm now proceeding to request certificates.

I'm also testing the InstServer export. Overall, I found two issues that will need addressing.

(1) With InstServer, I found that the RADPROTOS definition in edumanage/models.py has the TLS-signalling values (TLS, DTLS) commented out: https://github.com/grnet/djnro/blob/master/edumanage/models.py#L285

Wondering whether these should be just plain uncommented now - or whether there would be a need to keep them hidden. I can see they were just added commented out in 6e7ea05a.

But uncommenting at least the TLS (and possibly also DTLS) values sounds as a straightforward way to me. (Not sure about the TCP value, could be uncommented as well for completeness). Any thoughts on this @zmousm ?

(2) I also hit an issue where eDB2 wasn't picking up changes because the ts value wasn't changing. In case of the RealmServer objects, that was because I added them directly as new objects linked to the Realm object. When I later updated the Realm object directly and ts was updated, CAT picked up the changes. Also, when adding new RealmServers from the /admin UI for editing a Realm object (adding the servers as new rows), the ts was updated, just not when adding the RealmServer objects directly via their standalone forms. This on its own can be left as a quirk that NRO admins should be aware of.

However, in case of the institution.xml export, we should be producing a ts that reflects the last time anything that contributes to the institution data was changed. Without relying on end-users (institutional admins) to understand the quirks of the application - anytime they hit a Save button, the change should propagate.

I see certain object types (incl. InstServer) have their own ts value.

From this, the logical extension would be to calculate the exposed institution.ts as max over InstitutionDetails.ts and all subordinate objects, incl. InstServer.ts and ServiceLoc.ts.

Not all subordinate objects have ts: these do:

• InstServer
• MonProxybackClient
• ServiceLoc
• InstitutionDetails
• CatEnrollment ... but that leaves out at least InstRealm and Institution's contacts.

It would be possible to add ts also to these objects, but that would still leave unhandled a change that would be removing a subordinate object (deleting a realm, inst-server).

Another option for making sure timestamps are updated is to explicitly "touch" the InstDetails object from all methods handling a change from the Manage UI. I.e., whenver adding, editing or deleting a new contact, realm, instserver,... load and save the InstDetails object to update ts. That would also keep the code otherwise simple (no changes to model, no changes to how ts is exposed).

Your thoughts on this @zmousm ?

I see this elaboration onts values seems to be rather outgrowing the original scope of this PR.

We might close this PR off and merge it, adding support for exposing server information to eDB2 - and create a subsequent issue/PR for dealing with ts updates?

The only thing I think should happen still within this PR is bringing the data migration in line with the model definition. @ghalse , would you mind commiting the proposed changes? Or, alternatively, I could push them into your branch if you'd be OK with that (just tick "Allow edits from maintainers". ).

@zmousm , @ghalse , would you be OK with this way of moving forward?

Please let me know.

Cheers, Vlad

[ghalse]

@vladimir-mencl-eresearch I'm travelling so haven't really had a chance to look at this. I'd be happy for you to merge your migration changes, but don't seem to be able to allow edits in this PR. Are you sure it's not already enabled?

[vladimir-mencl-eresearch]

Sorry, couldn't add the commits directly, so I'm instead sending them via a PR to a PR: tenet-ac-za#2

[ghalse]

Sorry, couldn't add the commits directly, so I'm instead sending them via a PR to a PR: tenet-ac-za#2

That seems to have worked. I realise I have the second part of your patch as a separate local change in our branch already.

[vladimir-mencl-eresearch]

Thanks for the quick action @ghalse !

What do you think of this PR as a whole now, @zmousm ?

I have it running in PROD and I have eDB picking up the changes - and CAT now sees both NRO and Member servers as TLS/RadSec enabled and offers to issue certificates for them.

While there's extra work that could be done to properly handle ts values, this can be done as subsequent work and I think this PR could be merged as completed piece of work that exposes the server names.

Any objections to this, @zmousm ?

I'd otherwise aim to merge this next week....

Cheers, Vlad

[vladimir-mencl-eresearch]

I have now successfully tested CAT can actually issue certificates based on this data - merging now.

[vladimir-mencl-eresearch]

Thanks @ghalse for the contribution!