Two fixes related to the Instserver M2M changes (reinstate #7) #12
Conversation
| if not 'ertype' in self.cleaned_data: | ||
| raise forms.ValidationError(_('The Type field is required to validate this field.')) | ||
| ertype = self.cleaned_data['ertype'] | ||
| if ertype in [1,3]: |
There was a problem hiding this comment.
This field is actually required even if the institution/server is SP only, because NRO servers may need to know whether they should handle accounting packets. We want to deprecate accounting and thus get rid of RADTYPES in the future, but this is how things stand for now. So I believe you should remove any 'ertype' checking in this function.
There was a problem hiding this comment.
Sorry, scratch this comment, it is not true: there is no use of rad_pkt_type for SP only servers/institutions. Your approach is fine and it is more appropriate than checking institution ertype. Eventually we should also remove/hide this field from the form for such cases.
|
Hi Vladimir. Thanks for this. Apart from the line note, LGTM! Please rebase on branch next:
|
(cherry picked from commit 15691d3)
|
Rebase on next and it's ready to merge. |
Update code linking from Institution to InstServer to reflect changes in the model introduced in 15691d3 - instead of instserver_set, use the relation name "servers" now explicitly given in the model.
The validation code was relying on each server having just one institution. With the M2M mapping of Institutions and Servers, change the logic in clean_ertype to iterate over all institutions and check the server ertype complies with all of the institutions / their ertype values. For other server values required for an IdP (auth_port, acct_port, pkt_type), check them against this server er_type. When fetching the ertype of this server, check it has been set to avoid an embarassing KeyError.
vladimir-mencl-eresearch
force-pushed
the
fix-instserver-M2M
branch
from
e895de6 to
79a6778
Compare
|
Re-submitted as #13 with next as the target for merging. |
Hi Zenon,
These are the fixes I originally pushed in #7 - now in a separate branch.
Cheers,
Vlad