Bug 54812 - [UCS@school] Date validation error is misleading
Summary: [UCS@school] Date validation error is misleading
Status: CLOSED FIXED
Alias: None
Product: UCS@school
Classification: Unclassified
Component: Ucsschool-lib
Version: UCS@school 5.0
Hardware: Other Linux
: P5 normal
Target Milestone: UCS@school 5.0 v2
Assignee: Carlos García-Mauriño
QA Contact: Tobias Wenzel
URL: https://git.knut.univention.de/univen...
Keywords:
Depends on: 54668
Blocks: 54973
  Show dependency treegraph
 
Reported: 2022-06-01 07:22 CEST by Daniel Tröder
Modified: 2022-07-15 08:31 CEST (History)
7 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 3: Simply Wrong: The implementation doesn't match the docu
Who will be affected by this bug?: 2: Will only affect a few installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.069
Enterprise Customer affected?:
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): Error handling
Customer ID: 10568
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Tröder univentionstaff 2022-06-01 07:22:55 CEST
+++ This bug was initially created as a clone of Bug #54668 +++

Upon creating a school user with an expiration date later than 31.12.2099 the error "Not a valid Date" is thrown, but the date format is indeed right (maybe our syntax expects something 20**?).

For reference:

Unprocessable Entity: {'userexpiry': 'The property userexpiry has an invalid value: Invalid syntax. Account expiry date: Not a valid Date'}
Comment 1 Carlos García-Mauriño univentionstaff 2022-06-14 11:15:58 CEST
GitLab issue: https://git.knut.univention.de/univention/ucsschool/-/issues/748
Comment 2 Carlos García-Mauriño univentionstaff 2022-06-14 16:10:47 CEST
The original bug (Bug #54668) was merged and then reverted in UCS because the new error message (specifying the valid ranges of day month and year) was considered to be even more confusing than the original one (“Not a valid date”) for some people.

Then changes in UCS@School stayed. Issue https://git.knut.univention.de/univention/ucsschool/-/issues/691 and MR https://git.knut.univention.de/univention/ucsschool/-/merge_requests/94.

But the bug was reopened (new issue https://git.knut.univention.de/univention/ucsschool/-/issues/748) by Daniel Tröder who requested the following changes:

```
* Please fix the syntax class for "expiration_date" in ucsschool.lib.models.attributes.UserExpirationDate. It should be the same as it is defined in UDM (management/univention-directory-manager-modules/modules/univention/admin/handlers/users/user.py →  "userexpiry"): "date2".

-> fix this in 4.4, 5.0 and kelvin.

* The syntax class for "birthday" is "iso8601Date". That has no limit on the time range. Please revert that in Kelvins OpenAPI definition.
```

Currently the discussion is taking place at https://git.knut.univention.de/univention/ucsschool/-/issues/748 and there are three MRs for the requested changes:

* UCS@School 5.0: https://git.knut.univention.de/univention/ucsschool/-/merge_requests/111
* UCS@School 4.4: https://git.knut.univention.de/univention/ucsschool/-/merge_requests/112
* Kelvin REST API: https://git.knut.univention.de/univention/components/ucsschool-kelvin-rest-api/-/merge_requests/8
Comment 3 Carlos García-Mauriño univentionstaff 2022-06-17 04:58:47 CEST
After discussing what this issue is about, here is a summary of what needs to be done:

* Revert year constraints in the Kelvin swagger UI docs of the `birthday` parameter since the syntax is iso8601Date. The regression was intouduced in https://git.knut.univention.de/univention/ucsschool/-/merge_requests/94.
* In Kelvin, only accept YYYY-MM-DD for both `userExpiry` and `birthday`. Currently those attributes are part of a pydantic model and have `datetime.date` type so pydantic handles the casting from string to date and supports other formats.
* Create error messages that specify if the problem is the format or the range and what the accepted format or range is.
* Check if the kelvin client needs to be updated.
* In Kelvin, mention the valid range for userExpiry in the swagger UI docs and in the rst ones.


Everything applies to Kelvin (fastapi + school lib), UCS@School lib 4.4 and 5.0 unless stated otherwise.
Comment 4 Carlos García-Mauriño univentionstaff 2022-06-23 15:07:29 CEST
What was done:

* `birthday` user attribute's syntax is now iso8601Date again (no year range restricion).
* Kelvin only accpets YYYY-MM-DD for both `userExpiry` and `birthday`.
* Kelvin returns an error message informing about the valid date format.
* Kelvin returns error code and message if the year of `userExpiry` is out of range.
*Kelvin Swagger UI comments inform about date format and range.

What's missing (out of scope):

* Improve the date validation error directly in the `date2` syntax class in UCS. Reopened Bug #54668.

Reference:

* UCS@School 5.0: https://git.knut.univention.de/univention/ucsschool/-/merge_requests/111
  ucs-school-lib: 13.0.18A~5.0.0.202206201929
  advisory: 4aee5db2
* UCS@School 4.4: https://git.knut.univention.de/univention/ucsschool/-/merge_requests/112
  ucs-school-lib: 12.2.38A~4.4.0.202206211321
* Kelvin REST API: https://git.knut.univention.de/univention/components/ucsschool-kelvin-rest-api/-/merge_requests/8
  will probably be released in 1.5.5
Comment 5 Tobias Wenzel univentionstaff 2022-06-24 08:34:47 CEST
all ok, Verify

ucsschool 4.4/5.0

- changelog ok
- advisory ok
- merge ok
- jenkins happy

kelvin:

- changelog ok
- jenkins + pipeline happy
- already released with 1.5.5
Comment 6 Tobias Wenzel univentionstaff 2022-07-15 08:31:12 CEST
UCS@school 5.0 v2 has been released.

https://docs.software-univention.de/changelog-ucsschool-5.0v2-de.html

If this error occurs again, please clone this bug.