Closed SebastianCelejewski closed 8 years ago
Uważam, że metoda register w przypadku poprawnej rejestracji użytkownika, też powinna zwracać token, aby można było od razu przejść do skanowania kodów QR.
2015-12-01 11:26 GMT+01:00 Sebastian Celejewski notifications@github.com:
Proponuję kontynuować code review i dyskusję tutaj.
You can view, comment on, or merge this pull request online at:
https://github.com/openpkw/openpkw-weryfikator/pull/15 Commit Summary
- dodany rest login
- dodano findByEmailAndPassword
- dodano /login i /register
- dodano /login i /register
- zmiana w /login i /register
- zmiana w /login i /register
- zmiana w /login i /register
- zmiana w /login i /register
- Merge master into Waldek
- Automatyczne sformatowanie kodu zgodnie z code conventions dla OpenPKW
- Wprowadzenie konfiguracji dla automatycznych testów akceptacyjnych
- Usunięcie błędów, dodanie testów automatycznych, zwracanie błędów przy pomocy kodów HTTP
- Zmiana zwrotu z web serwisu ze String na JSON. Dodanie słownika kodów błędów. Dodanie testów automatycznych
- Dodanie kilku przykładowych unit testów dla walidacji
- Więcej unit testów
- Dodanie walidatora dla requestu authorize oraz kilku jego unit testów
File Changes
- M openpkw-model/pom.xml https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-0 (91)
- M openpkw-model/src/main/java/org/openpkw/model/entity/PeripheralCommittee.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-1 (261)
- M openpkw-model/src/main/java/org/openpkw/model/entity/PeripheralCommitteeAddress.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-2 (226)
- M openpkw-model/src/main/java/org/openpkw/model/entity/User.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-3 (266)
- A openpkw-model/src/main/java/org/openpkw/model/entity/UserDevice.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-4 (77)
- M openpkw-repositories/pom.xml https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-5 (101)
- A openpkw-repositories/src/main/java/org/openpkw/repositories/UserDeviceRepository.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-6 (12)
- M openpkw-repositories/src/main/java/org/openpkw/repositories/UserRepository.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-7 (25)
- M openpkw-rest/pom.xml https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-8 (241)
- A openpkw-rest/src/main/java/org/openpkw/web/Autorize.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-9 (65)
- A openpkw-rest/src/main/java/org/openpkw/web/Token.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-10 (24)
- A openpkw-rest/src/main/java/org/openpkw/web/UserRegister.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-11 (76)
- M openpkw-rest/src/main/java/org/openpkw/web/configuration/AppConfig.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-12 (34)
- M openpkw-rest/src/main/java/org/openpkw/web/configuration/AppInitializer.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-13 (48)
- M openpkw-rest/src/main/java/org/openpkw/web/configuration/MVCConfig.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-14 (60)
- A openpkw-rest/src/main/java/org/openpkw/web/controllers/LoginController.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-15 (127)
- A openpkw-rest/src/main/java/org/openpkw/web/validation/LoginControllerRequestValidator.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-16 (42)
- A openpkw-rest/src/main/java/org/openpkw/web/validation/RestClientErrorMessage.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-17 (38)
- A openpkw-rest/src/main/java/org/openpkw/web/validation/RestClientException.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-18 (24)
- M openpkw-rest/src/main/resources/META-INF/spring/openpkw-context.xml https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-19 (61)
- M openpkw-rest/src/main/webapp/WEB-INF/web.xml https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-20 (11)
- A openpkw-rest/src/test/java/org/openpkw/web/controllers/EchoControllerTest.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-21 (52)
- A openpkw-rest/src/test/java/org/openpkw/web/controllers/LoginControllerTest.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-22 (93)
- A openpkw-rest/src/test/java/org/openpkw/web/validation/AuthorizeBuilder.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-23 (22)
- A openpkw-rest/src/test/java/org/openpkw/web/validation/UserRegisterBuilder.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-24 (38)
- A openpkw-rest/src/test/java/org/openpkw/web/validation/When_validating_user_authorization_request.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-25 (34)
- A openpkw-rest/src/test/java/org/openpkw/web/validation/When_validating_user_registartion_request.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-26 (53)
- M openpkw-test/pom.xml https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-27 (6)
- A openpkw-test/src/test/java/org/openpkw/weryfikator/rest/Configuration.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-28 (10)
- M openpkw-test/src/test/java/org/openpkw/weryfikator/rest/When_sending_GET_request_to_root_url.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-29 (2)
- M openpkw-test/src/test/java/org/openpkw/weryfikator/rest/When_sending_POST_request_to_echo_url.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-30 (2)
- A openpkw-test/src/test/java/org/openpkw/weryfikator/rest/login/When_registering_new_user.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-31 (92)
- A openpkw-utils/src/main/java/org/openpkw/utils/StringUtils.java https://github.com/openpkw/openpkw-weryfikator/pull/15/files#diff-32 (13)
Patch Links:
- https://github.com/openpkw/openpkw-weryfikator/pull/15.patch
- https://github.com/openpkw/openpkw-weryfikator/pull/15.diff
— Reply to this email directly or view it on GitHub https://github.com/openpkw/openpkw-weryfikator/pull/15.
Sebastian proponuje te resty rozpisac w swaggerze. Czyli jakie beda metody co beda przyjmowaly i co zwracaly. Jezeli masz juz jakies pomysl na strukture wklej to tutaj to zobacze i rozrysuje to w swagerze.
Zastanawiam się jeszcze nad URLami do tych web serwisów.
Teraz mamy: /authorize/register /authorize/login
Wydaje mi się, że REST powinien mieć urle bardziej w tym stylu: GET /users (pobiera listę) GET /users/{id} (pobiera jednego) POST /users (tworzy nowego) DELETE /users/{id} (usuwa jednego)
POST /sessions (logowanie, czyli tworzenie nowej sesji) GET /sessions (pobieranie listy sesji)
itd.
Co o tym sądzicie?
Tylko wtedy jest problem z identyfikatorami. Bo u nas identyfikatorem użytkownika jest jego adres e-mail. I wtedy URL wyglądałby tak:
/users/Jan.Kowalski@poczta.pl
Ogolnie Sebastian ja mysle ze powinnismy wypracowac taki spojny dokument jak te urle beda u nas wygladały przy Restach w zwiazku z tym dodam zadanie na trello do realizacji na opracowanie takiej koncepcji. I moze do celow koncepcyjnych uzyjmy forum na git. Założe dziś tak temat ( w sensie issue). Issue jest tutaj: https://github.com/openpkw/openpkw-weryfikator/issues/16
Moim zdaniem nie ma tutaj problemu, ponieważ będziemy znali klucz publiczny RSA serwera, więc zaszyfrujemy identyfikator użytkownika (email), wówczas /users/encrypted_email lub /users/encrypted_email/hash(url) Następnie serwer swoim kluczem prywatnym zdeszyfruje encrypted_email
W dniu 2 grudnia 2015 08:01 użytkownik Mrozi notifications@github.com napisał:
Ogolnie Sebastian ja mysle ze powinnismy wypracowac taki spojny dokument jak te urle beda u nas wygladały przy Restach w zwiazku z tym dodam zadanie na trello do realizacji na opracowanie takiej koncepcji. I moze do celow koncepcyjnych uzyjmy forum na git. Założe dziś tak temat.
— Reply to this email directly or view it on GitHub https://github.com/openpkw/openpkw-weryfikator/pull/15#issuecomment-161202625 .
Dzięki za uwagi.
Proponuję kontynuować code review i dyskusję tutaj.