search

erasmus-without-paper / ewp-specs-api-iias

Specifications of EWP's Interinstitutional Agreements API.
MIT License
4 stars 13 forks source link

Excluding certain fields when calculating conditions-hash #48

Closed kamil-olszewski-uw closed 1 year ago

kamil-olszewski-uw commented 3 years ago

There are fields in the cooperation coditions that are not in the IIAs template. Those are:

Recently, there has been the idea that certain fields should be removed from the conditions-hash computations, because changing these fields would change the hash, but they do not contain information that should affect the validity of the agreement.

We believe that this should apply to fields sending-contact and receiving-contact, as the contact details of cooperation coordinators can indeed change.

However, we believe that the hash should take into account the field sending-ounit-id and receiving-ounit-id, because the faculties implementing cooperation conditions are unlikely to change. And if this happens (or the name of the organizational unit changes due to changes in the structure of the university), a new agreement should be signed in such a case.

Please comment if you think we should do it differently.

fmapeixoto commented 3 years ago

In my opinion, the contacts shouldn't be calculated because, as referred by Kamil, "...do not contain information that should affect the validity of the agreement.".

Regarding the OUnits in the cooperation conditions, I have 2 use cases that I understand may exists or not:

  1. in case a OUnit is specified at the partners level, no more different OUnits should appear in the cooperation conditions right? if affirmative they can: a) be present (required) b) present (optional) c) none at all (because they are already at the top
  2. in case no OUnit is specified at the partner level, then the possibility of having OUnits in the Cooperation Conditions is something that may be needed in much cases. In order to make easier for specifications, the optional parameters may be the best option.
kamil-olszewski-uw commented 3 years ago
  1. in case a OUnit is specified at the partners level, no more different OUnits should appear in the cooperation conditions right? if affirmative they can: a) be present (required) b) present (optional) c) none at all (because they are already at the top

There may be situations when e.g.:

Of course, an erroneous situation may arise when, for example, faculty is specified at the partner level, and other faculties are specified at the cooperation conditions level. But it is a business error that should be noticed by the user and not by the system.

fmapeixoto commented 3 years ago

And one more remark, does it make sense, per cooperation condition, to have it 0-unbounded? Shouldn't it be just 0-1 per cooperation? Just asking because I don't know practical use-cases of these.

umesh-qs commented 3 years ago

Restricting to a maximum of 1 can also resolve https://github.com/erasmus-without-paper/ewp-specs-api-iias/issues/29. This was closed today without any solution.

EvelienRenders commented 3 years ago

I don't know if this is relevant, but the Erasmus Dashboard right now only allows one OUnit per Agreement (so not even on the cooperation level). Is this something that EUF should 'fix'?

For example: @umesh-qs, it might prove weird when one of your MoveOn users starts an agreement with 4 cooperation conditions that each has a different OUnit, and it comes into the Dashboard as 4 separate agreements.

What happens if the Dashboard user then changes things on an agreement level, and sends it back? What does the MoveOn user see then?

As for your remark @fmapeixoto

And one more remark, does it make sense, per cooperation condition, to have it 0-unbounded? Shouldn't it be just 0-1 per cooperation? Just asking because I don't know practical use-cases of these.

It depends; right now the Dashboard users are making up OUnits for 'institution-wide' agreements - because they cannot choose multiple OUnits per Agreement in the Dashboard. That means that we have 7 faculties + 1 institution-wide OUnit.

That is not a problem per se, but might not be the perfect way to use these Units...

umesh-qs commented 3 years ago

@EvelienRenders I did not understand whether you are supporting multiple cooperation conditions or not. But it is certainly possible. How it is implemented in the Dashboard is something that is specific to Dashbord and if it is implemented the way you described then personally that is not the best way.

But that is not what is there under the issue https://github.com/erasmus-without-paper/ewp-specs-api-iias/issues/29. What I was asking is that when there is no limit on the number of cooperation conditions then how do I uniquely identify each cooperation condition. Restriction to max 1 for each type can resolve the issue.

EvelienRenders commented 3 years ago

@umesh-qs Forgive my non-technical question ;) I'm an information manager at a Dutch university and not working directly on the development of the connection to our SIS (Osiris, developed by CACI). I'm trying to get a grip of the issues that are still ongoing - and I understand your issue #29 - although unfortunately, I can only add an HEI's perspective on these issues.

I find it comforting to hear that you think the Dashboard's implementation is not the best because personally, I'm thinking that as well - I will send EUF the feedback via their support portal once it is online again...

Thank you for all the hard work that is going into this on your end, but also on the ends of the other 3rd party providers - it's much appreciated (although not many might let you know during the 'trial and error' phase 😉)

pleys commented 3 years ago

@kamil-olszewski-uw I think we should leave contacts out of conditions hash. I see a great advantage of having contacts connected the the agreements, e.g. for sharing the academic coordinator of a given agreements (who can be the responsible person for signing learning agreements) but this person can change every year and we don't want to re-approve our agreement every year.

pleys commented 3 years ago

The organisational units discussion is somewhat more complex and the discussion above points to the issue of grouping agreements that I raised earlier.

umesh-qs commented 3 years ago

@kamil-olszewski-uw I am sorry, I did not understand, what are we trying to solve here by adding/removing fields from the calculation of hash?

kamil-olszewski-uw commented 3 years ago

I am sorry, I did not understand, what are we trying to solve here by adding/removing fields from the calculation of hash?

The point is that the mobility coordinators are not part of the agreement. They would not be on a traditional paper agreement. However, since the coordinators are part of the cooperation conditions in API, the change of the coordinator would make the hash, which was the basis for accepting the agreement, obsolete. And the change of coordinator is something common. Therefore, we want sending-contact and receiving-contact not to be taken into account when hash is calculated.

fmapeixoto commented 3 years ago

I believe the discussion here should be directly related to whether we remove some fields from the calculation of the Hash, and or they should exist, and if so, in which circumstances.

For the IIA ecosystem (all APIs) to work properly, all the systems (ownsys and providers) have to guarantee that they are able to comply with all the fields (at least for the calculation of the hash) and have proper ways of storing and sending the exact same values, in the same order, so the hash match every time. I don't think we should discuss here already what is supported by whom because what is agreed here should be implemented to comply.

I agree with Paul regarding the contacts. Regarding the ounits, I leave the he discussion to the ones who have more deep knowledge about all the use-cases and needs.

umesh-qs commented 3 years ago

@kamil-olszewski-uw I think too much weightage is given to hash. Your scenario is frequent changes to agreement contact. Others are talking about ounit. I have a problem that recent API changes will change the hash of all the agreements exchanged via EWP network. Are our clients supposed to approve all of them again now and again in future if more changes are done?

fmapeixoto commented 3 years ago

@umesh-qs that is actually a good question which I believe is related because what else is proof that a specific agreement has been "signed" if the hashes don't match on both sides? Perhaps coming back to the topic of having a digital signature in the XML somehow fixes everything. But is the same of having the fields and not being able to change any of them after "signing" (including contacts).

kamil-olszewski-uw commented 3 years ago

I have a problem that recent API changes will change the hash of all the agreements exchanged via EWP network. Are our clients supposed to approve all of them again now and again in future if more changes are done?

We are currently in the infant stage of the IIAs Approval API. I cannot speak for others, but we do not have any digitally approved IIA yet, not including test agreements. And even if some agreements between partners using EWP have been approved via EWP, there should still be signed agreements on paper. The most important thing is to meet the specification on the deadlines set by the EC regarding agreements, which will be signed only digitally.

kamil-olszewski-uw commented 3 years ago

There are a few important issues in this thread - related, but still separate, so we have a slight chaos here. I will try to answer here to unanswered questions:

1. Number of organizational units with cooperation conditions:

We understand that @fmapeixoto asked about the 0-unbounded to 0-1 change in the context of organizational units under cooperation conditions, and not in the context of the cooperation conditions themselves. We don't remember who postulated the unbounded organizational units and think we can limit them to 0-1 as long as no one objects.

2. Number of cooperation conditions of one kind:

We cannot change cooperation conditions from 0-unbounded to 0-1. A simple use case where the unbounded option matters:

3. Differences between Dashboard and API:

@EvelienRenders - If something in the Dashboard currently looks different than the final API shape that we collectively agree to, we are convinced that the Dashboard will be adapted to the API structure.

kamil-olszewski-uw commented 3 years ago

SUMMARY (our proposed changes to IIAs API based on our discussion):

  1. We will remove sending-contact and receiving-contact from the calculation of the hash.

  2. We will NOT remove sending-ounit-id and receiving-ounit-id from the calculation of the hash.

  3. We will change maxOccurs of sending-ounit-id and receiving-ounit-id from unbounded to 1.

  4. We will NOT change max Occurs of student-studies-mobility-spec, student-traineeship-mobility-spec, staff-teacher-mobility-spec and staff-training-mobility-spec from unbounded to 1.

  5. We will NOT force a specific number of digits in the isced-f-code, but in the commentary to this element we will strongly recommend that the ISCED code should be four-digit, because this way it will be compliant with the requirements of Mobility Tool+ apllication, which only accepts four-digit codes for mobilities (see issue: #49).

umesh-qs commented 3 years ago

Please suggest how to handle hash/approval for existing agreements in the EWP network.

umesh-qs commented 3 years ago

@kamil-olszewski-uw Not sure if this was discussed before releasing the new IIA API. How do we handle hash changes for the approved agreements in EWP?

EvelienRenders commented 3 years ago

With regards to point 3 - we found that this would be a bit restrictive. Could someone explain elaborate if this is a technical requirement?

  1. We will change maxOccurs of sending-ounit-id and receiving-ounit-id from unbounded to 1.
umesh-qs commented 3 years ago

@kamil-olszewski-uw I think IIA API changes have been released, without considering all the points. I would like to know what is the process for approving the changes for an API and who takes the final call to release the changes even if there are unanswered questions?

kamil-olszewski-uw commented 3 years ago

We will change maxOccurs of sending-ounit-id and receiving-ounit-id from unbounded to 1.

With regards to point 3 - we found that this would be a bit restrictive. Could someone explain elaborate if this is a technical requirement?

Do you really have individual cooperation conditions dedicated to several (but not all) organizational units? We have several thousand agreements with universities from the EU and almost as much with the rest of the world, and it was always enough for us to indicate one unit or no unit at all.

kamil-olszewski-uw commented 3 years ago

Not sure if this was discussed before releasing the new IIA API. How do we handle hash changes for the approved agreements in EWP?

Do you mean (1) changes to the existing agreement due to the modification of the cooperation conditions or (2) obsolescence of the hashes calculated so far due to the change in the method of calculating the hash?

Ad 1. We thought about it and many voices were in favor of the solution that the change in cooperation conditions should result in signing a new agreement.

Ad 2. As I wrote earlier, we are currently in the infant stage of the IIAs Approval API. First stable version was released few years ago. It is difficult to consider earlier versions as an unchanging basis without the risk of numerous backwards incompatible changes

umesh-qs commented 3 years ago

@kamil-olszewski-uw So this is an official stand that because of new changes to the API structure all the existing agreements are not valid anymore until approved again? Is it ok to communicate this to our clients once we release the new changes?

umesh-qs commented 3 years ago

@kamil-olszewski-uw Awaiting your response to my last comment, please.

janinamincer-daszkiewicz commented 3 years ago

This is not true that because of the new changes to the API structure all the existing agreements are not valid anymore. First of all they have already been approved. No need to approve them again. Second, they will stay approved under the old API. It is natural that the new APIs will show up and will have to be implemented. We have nothing else to add so please do not push.

umesh-qs commented 3 years ago

"We have nothing else to add so please do not push.". This is a very harsh comment. I was just trying to get some answers since I have to respond to multiple stakeholders in my organization. And I don't have the option to tell them not to push me, since EWP API designers/approvers don't like asking questions.

Let me come back to the issue of hash change, with an example

  1. HEI A creates and shares an IIA with HEI B.
  2. HEI B pulls the IIA data.
  3. HEI B validates the hash and approves the IIA of HEI A.

Now both HEI A and HEI B implements IIA API version 6.0

  1. HEI B pulls the data of the same IIA from HEI A.
  2. Since there is a change in hash calculation logic in the new API, HEI B will get a new hash and has to approve again.

Can any of the IIA service provider, suggest ways, where our clients, don't have to re-approve the already approved agreement in the above scenario?

sascoms commented 3 years ago

If I have not misunderstood the question:

There two ways:

  1. To keep the approval status in your systems. This way, even if the hash or the api version changes, you still have a previous agreement set as approved in the past. And no need for a re-approval operation. Or even if the API version changes, these IIAs are already approved. No need to re-approve (if there is no IIA content updated/changed)

  2. To send CNR to the partner and re-initiate the approval process.

In my opinion, already approved IIAs do not need to go through a new approval check or process as the re-approval process should apply only if one of the partners change any of the content or the valid years or similar data in the IIA.

However, if the IIA API version sets new required fields and these fields are missing in the existing IIAs and the provider systems require these fields to exist for approval process or mobilities, then #2 option should definitely be applied.

fmapeixoto commented 3 years ago

This will be a problem over time, not only from 5 to 6 or 4 to 5. In my opinion, adding the signature field in the XML will make it irrefutable and, if signed by both, APIs can change the whole structure that the signature of the saved XML is always valid and you can keep it in the DB. Instead of calculated hashes, signatures of both parties are enough. The past ones, we have to trust that partners support older versions, or at least keep the older hashes in the approvals API.

kamil-olszewski-uw commented 3 years ago

As Francisco wrote, this problem may reappear many times in the future - for example, when the EC decides to introduce completely new elements to the cooperation conditions.

We plan to add signatures in the future. But this is a more general issue that also applies to other APIs.

Currently, for example, you can use such a solution: If the institution wants the approve agreement only once - with a hash that will never expire and can always be used to check whether the agreement has not changed - then the institution may keep an information in the system, which the IIAs API version was used during approval of the agreement, and the system may have hash calculation algorithms implemented for different versions of the IIAs API.

fmapeixoto commented 3 years ago

If, for already approved IIAs, we just check the approvals API for the partner hash and our DB, they should match, we don't need to recalculate it.

kamil-olszewski-uw commented 3 years ago

If, for already approved IIAs, we just check the approvals API for the partner hash and our DB, they should match, we don't need to recalculate it.

I'm afraid I didn't understand your idea :-) Could you please present it as a simple scenario?

fmapeixoto commented 3 years ago

The API IIAs Approval have the following entries: approval> iia-id>82a94c1684ce-947c-7ac9-cd7f-287f065a/iia-id conditions-hash>3d3310935590256ecff21549b45eb4e1662b0f2a2aafcb7d91ea5f271f4d9e07/conditions-hash> /approval> When you retrieve a partner IIA (with IIA Get):

If they match there is no need to recalculate hash of a particular IIA as it is already approved, independent of the version, right?

kamil-olszewski-uw commented 3 years ago

What exactly would a partner's hash be compared to in "it's own DB"? To a previously received (during approving) partner hash, which we keep in our DB? The positive result of the hash comparison will not be any proof that the partner has not changed anything in the cooperation conditions in the meantime. After all, a partner can send cooperation conditions with an incorrect hash. This is why it is necessary to calculate the hash during approving and therefore it would be necessary later if we would like to check if there were any illegal changes after the approval.

EvelienRenders commented 3 years ago

We will change maxOccurs of sending-ounit-id and receiving-ounit-id from unbounded to 1.

With regards to point 3 - we found that this would be a bit restrictive. Could someone explain elaborate if this is a technical requirement?

Do you really have individual cooperation conditions dedicated to several (but not all) organizational units? We have several thousand agreements with universities from the EU and almost as much with the rest of the world, and it was always enough for us to indicate one unit or no unit at all.

Thanks for the question Kamil!

We do have so-called Joined Faculty Agreements - where we normally put multiple faculties on one condition. However, I've discussed this internally, and it would not be a problem for us to separate them over two cooperation conditions in the future - one for each OUnit (faculty).

frangarcj commented 3 years ago

Trying to not be too much off-topic, I have some questions arising from this discussion:

  1. Is there any recommendation on what api version (client/server) to maintain? how many?
  2. Is there any roadmap on api version increments?
  3. Also a lot of HEIs are in the dashboard, do you know if they have a roadmap for api versions?

We were finishing our IIAs 3.0 implementation in our tool at the University of Almería and suddenly versions 4, 5 and 6 appeared, so we are a little confused.

Thanks in advance

janinamincer-daszkiewicz commented 3 years ago

Is there any recommendation on what api version (client/server) to maintain? how many?

The most recent. As many as it makes sense. If all your partners have upgraded to the newer versions there is not needed to keep suport for the older ones.

Is there any roadmap on api version increments?

We try not to increment version without specific need. Recent changes were initiated by the new templates of LA and IIA announced by the commision.

Also a lot of HEIs are in the dashboard, do you know if they have a roadmap for api versions?

I am sure that the developers of the Dashboard and OLA will upgrade to the most recent versions as soon as possible. I can not give any dates on their behalf.

umesh-qs commented 3 years ago

@janinamincer-daszkiewicz

  1. Yes it makes sense to keep the most recent version of API. But, is there a cutoff date by when everyone in EWP network has to move to the latest version?
  2. Are all the recent changes to the API, result of the new announcement by the commission?
janinamincer-daszkiewicz commented 3 years ago

Yes it makes sense to keep the most recent version of API. But, is there a cutoff date by when everyone in EWP network has to move to the latest version?

I am not in the authority to decide about such dates.

Are all the recent changes to the API, result of the new announcement by the commission?

Some are the results from other discussions, in Github, by e-mails. We ask developers to put the information in Github, to make it public.

umesh-qs commented 3 years ago

Ok, so who has the authority to decide on putting the cut off date? Will contact them.

On the changes based on Github discussions, emails ... who takes a call on that and what is the basis of it?

janinamincer-daszkiewicz commented 3 years ago

I don't know, may be DGEAC?

umesh-qs commented 3 years ago

@janinamincer-daszkiewicz

  1. Any specific person/team in DGEAC with whom the EWP API team is working for the requirement?
  2. On the changes based on Github discussions, emails ... who takes a call on that and what is the basis of it?
  3. Who all are part of the API design/release team? I assume at the moment that is it you and Kamil. Anyone else?
jpbacelar commented 3 years ago

Umesh, if you have questions about the EWP governance feel free to contact me. The purpose of this git is to provide a platform to carry out technical discussions with the EWP community, and I sense your current interests lie elsewhere.

As for decision-making we are very lucky to have a team that spans several universities, and that has a pretty decent track record when it comes to take into consideration not just technical input (e.g. from the git) but also policy and user requirements. Of course your question is disingenuous, since you met most of these colleagues when attending EWP meetings...

umesh-qs commented 3 years ago

Hi Joao, Any questions raised or arguments posted here are specific to this topic and should be seen in that context. Let us stick to this topic and not divert by bringing in the credibility question. I don't think anyone here is questioning anyone's credibility, including me.

My interest lies with my organization and the clients using our product. If I am not getting proper answers, or getting rude/intimidating answers ("I don't know, may be DGEAC?", "We have nothing else to add so please do not push"), then yes, you might have to be involved and maybe frequently. And all I was looking at to find the right answers.

I was looking for answers to technical issues here. Other questions came up because of the responses received. I am happy to contact you directly for any concerns related to any API, if that is what seems right.

I still don't have a 100% working solution on how to handle approved agreements on the hash change when different service providers are using a different version of the same API. If there is no 100% solution and API changes are mandatory, then I feel, that has to be communicated accordingly. This will help us to communicate further to our clients so that they are aware of the impact of changes on their workflow.

jpbacelar commented 3 years ago

Hi Umesh,

Glad to hear we can get back on track, thanks for that. I can't solve the hash issue myself, but I have picked up several topics raised on this thread for the next project meeting - not least the matter of deprecating APIs, which is undoubtably pertinent. Hope we can borrow from this discussion to provide greater clarity and a more consistent framework for the dev teams.

umesh-qs commented 3 years ago

Hi, Can someone help me with hash calculation logic? For the below content. What should be the hash?

<ns40:cooperation-conditions>
<ns40:student-traineeship-mobility-spec>
<ns40:sending-hei-id>uni1</ns40:sending-hei-id>
<ns40:receiving-hei-id>uni2</ns40:receiving-hei-id>
<ns40:receiving-academic-year-id>2018/2019</ns40:receiving-academic-year-id>
<ns40:mobilities-per-year>1</ns40:mobilities-per-year>
<ns40:recommended-language-skill>
<ns40:language>en</ns40:language>
<ns40:cefr-level>C1</ns40:cefr-level>
</ns40:recommended-language-skill>
<ns40:isced-f-code>0313</ns40:isced-f-code>
<ns40:total-months>2</ns40:total-months>
<ns40:blended>false</ns40:blended>
</ns40:student-traineeship-mobility-spec>
</ns40:cooperation-conditions>

If I use https://emn178.github.io/online-tools/sha256.html, I get hash as a8d887727c7d2bf1ee97e42b828e469e226c62093308a5d820b5a9864f57acbf for data under cooperation-conditions section.

Please suggest if it is correct.

pmarinelli commented 3 years ago

Hello Umesh, I think that the xml fragment is not in canonical form as per exclusive c14n specs (please, anyone tell me if I'm wrong): there is no namespace declaration for prefix ns40, while I would expect such a declaration within the start tag of the cooperation-conditions element. In general, in order to conclude whether a cooperation-condition subtree is in canonical form or not, the whole iias-get-response document is needed. In your case, I think the missing ns declaration suffices to conclude that it is not. The hash calculation per-se is straightforward. Trickier is the generation of the canonical form of the cooperation-conditions subtree. I strongly recommend to use a library for that. We are developing our ewp connector in java and we opted for apache santuario for the canonicalization. We select the subtree to canonicalize through XPath.

umesh-qs commented 3 years ago

Hi @pmarinelli Thanks for responding.

Below is the xml fragment after converting to c14n specs

<ns40:cooperation-conditions xmlns:ns40="https://github.com/erasmus-without-paper/ewp-specs-api-iias/blob/stable-v3/endpoints/get-response.xsd">
<ns40:student-traineeship-mobility-spec>
<ns40:sending-hei-id>uni1</ns40:sending-hei-id>
<ns40:receiving-hei-id>uni2</ns40:receiving-hei-id>
<ns40:receiving-academic-year-id>2018/2019</ns40:receiving-academic-year-id>
<ns40:mobilities-per-year>1</ns40:mobilities-per-year>
<ns40:recommended-language-skill>
<ns40:language>en</ns40:language>
<ns40:cefr-level>C1</ns40:cefr-level>
</ns40:recommended-language-skill>
<ns40:isced-f-code>0313</ns40:isced-f-code>
<ns40:total-months>2</ns40:total-months>
<ns40:blended>false</ns40:blended>
</ns40:student-traineeship-mobility-spec>
</ns40:cooperation-conditions>

and below is the hash 4f0fc9f8904f63d687873e64e0d1674059aa33955535ebee7ed6e78efb9cd610

Does it look good now?

mkurzydlowski commented 3 years ago

Looks good to me. We will update our own EWP connector accordingly as we don't calculate the hash correctly currently.

frangarcj commented 3 years ago

If I'm not wrong the c14n form is without an XML namespace name

<cooperation-conditions xmlns="https://github.com/erasmus-without-paper/ewp-specs-api-iias/blob/stable-v3/endpoints/get-response.xsd"><student-traineeship-mobility-spec><sending-hei-id>uni1</sending-hei-id><receiving-hei-id>uni2</receiving-hei-id><receiving-academic-year-id>2018/2019</receiving-academic-year-id><mobilities-per-year>1</mobilities-per-year><recommended-language-skill><language>en</language><cefr-level>C1</cefr-level></recommended-language-skill><isced-f-code>0313</isced-f-code><total-months>2</total-months><blended>false</blended></student-traineeship-mobility-spec></cooperation-conditions>