[#80] GitHub authorization

[d1z3d]

No description provided.

[d1z3d]

@fey @Malcom1986 Hey!
I added authorization through GitHub. The link to the site is https://hexlet-correction-ac3h.onrender.com
I haven't figured out how to proceed with pr 159, so I created a new one.

[fey]

@d1z3d
good, but i cant open https://hexlet-correction-ac3h.onrender.com/
Can you check is it work?

[d1z3d]

@fey I redeployed a last commit. Now it works

[fey]

Still cant enter to deployed demo

[fey]

@d1z3d please add instruction to readme how to setup Github login

[d1z3d]

@fey i deployed to another service - https://d1z3d-hexlet-correction.onrender.com. Try it, please.
Okey, I'll add the instruction.

[fey]

@d1z3d cant load demo :C

Please add inctruction, we ll check auth on local

[fey]

This file is in Git, so changing it may cause us to accidentally run the changed file. Let's think about how we can avoid this. But in general, it could be that for local development we just set the right values, but for projection we set different values - will this work?

[d1z3d]

@fey Hey!

This file is in Git, so changing it may cause us to accidentally run the changed file. Let's think about how we can avoid this

To be honest, i dont get what do you mean. If you mean that other people will have to solve conflicts while pushing new changes from their own branches because files will be a different. So they can do merge it before push. I done it before i open the pr. To avoid this I can add a custom exception while building project to be done the check and don't change a README file. Will this option work for you?

But in general, it could be that for local development we just set the right values, but for projection we set different values - will this work?

I added 2 instructions: to local delopment and to deploy the project. A Dockerfile uses the production profile. See application-prod.yaml

[fey]

See, usually the files that need to be changed for local development are not versioned. These are different configuration files, settings that are specific to the environment and to the developer. For example, it could be the Github settings with which the connection is made. Then let's think about it, maybe just create a Github application for authorization locally.

[d1z3d]

@fey Hey!

I added a OAuth app for dev. Fixed README
It looks like:

[fey]

@d1z3d it only works for login, not for registration? I want to log in via Github, no accounts. But it won't let me in. No error handling.

In other services, we register a user if a user from Github is not registered on the service.

[d1z3d]

@fey hey!
I'll fix redirection. It works for registration. If you have no public email you will get this problem. You cannot sign in or up without public email.

[fey]

@d1z3d yea, i dont have public email. may be should be ssomething to fix that. Because i can register account with github on other services (can you try sicp, cv?)

[d1z3d]

I done that when I had a token from GitHub. With that I took private emails. I couldn't do it without the token.
Can you give advice how can I do that? Scopes don't affect on it.

[d1z3d]

I got private emails without public email but I used some custom requests for it via GitHub token. I will send changes while few days

[d1z3d]

@fey hey!
I fixed the redirection and added alert for login page. Can you check it? While you check I will read about how to test oauth2.
I saw an issue that names can be empty. I added TODO.

[Malcom1986]

@d1z3d
Hello! Is the work finished here? Or will there be tests here too? I can’t open the demo application, please check

[d1z3d]

Hey!

@fey, I add link on signup page.

@Malcom1986, I done with this issue. I read about testing oauth2 but I don't know how I can test it. I mean I can mock all requests to authenticate but I don't see reasons.

[fey]

@d1z3d you can read tests in hexlet-sicp with same task https://github.com/Hexlet/hexlet-sicp/blob/main/tests/Feature/Http/Controllers/Auth/Social/GithubControllerTest.php

[Malcom1986]

@d1z3d Приветствую, Андрей! Попробовал локально поднять приложение. Что-то не совсем коррекно работает. Регистрация через гитхаб проходит успешно. Потом выхожу из аккаунта и пытаюсь зайти через гитхаб. Ссылка на странице входа ведет опять на регистрацию

[Malcom1986]

Кстати при повторной регистрации получается 500 ошибка и не понятно что произошло. Нужно сообщить, что такой пользователь уже существует

[fey]

Кстати при повторной регистрации получается 500 ошибка и не понятно что произошло. Нужно сообщить, что такой пользователь уже существует

По идее логика входа должна быть такой

1. Создаем пользователя, Если его нет. Логиним, прописываем фио, ник с Гитхаба, почту. Пароль - хз (вероятно сами генерируем)
2. Если пользователь есть - логиним его

[d1z3d]

@Malcom1986 Максим, проблему понял. Я изменил логику для повторного входа. Т.е. если пользователь входит повторно, то сделал обновление данных УЗ. Видимо нужно достать пользователя по ID и уже менять его. Но по комментарию @fey Николая понял, что делать этого не нужно. Скорректирую обратно. Нужно было спросить прежде, чем делать) Попробовал локально с проверкой существующего пользователя. Проблемы при повторном входе не возникло. Сбросил кеш. Смог войти успешно снова.

Также уберу handler при неуспешной аутентификации. Он не отработал. Попробую добавить кастомный фильтр для отлова ошибок при регистрации.

@fey Николай, изучу тесты и добавлю по аналогии.

[d1z3d]

@Malcom1986, @fey, Максим, Николай, здравствуйте!
Внес правки, но вот с тестами беда. Не понимаю как сделать правильно.

[Malcom1986]

Автогенерирумые фалы типа AccountMapperImpl.java попали как-то в репозиторий. Они же должны быть в диретории build и соответственно не пушитья в репозиторий

[Malcom1986]

А вот такую статью смотрели по тестам? https://www.baeldung.com/spring-oauth-testing-access-control

[d1z3d]

А вот такую статью смотрели по тестам? https://www.baeldung.com/spring-oauth-testing-access-control

@Malcom1986 Максим, добрый день!
Нет, теперь я понял, что нужно делать. Спасибо!

[d1z3d]

@Malcom1986, Максим, внес правки. Автогенерируемые файлы создаются в "../src/main/generated/**". Я добавил правило для игнорирования файлов в .gitignore.

@fey, Николай, хотел бы подсветить сразу потенциальную проблему. В данном проекте есть функционал по изменению email-а. Это будет создавать проблемы.
Кейс:

1. Пользователь вошел на hexlet-correction с помощью социальной сети. В данном случае, это GitHub.
2. Пользователь изменил на GitHub свой email.
3. Он пробует войти повторно на hexlet-correction.
4. У пользователя будет ошибка при входе, потому что будет выполнен поиск пользователя по email и его не будет, т.к. email будет новым, а username останется прежним. Возникнет ошибка с contraint-ами, потому что username уникальный.

Также это актуально, если пользователь поменяет email на hexlet-correction, но не на GitHub. При этом пользователь не сможет войти под своей учеткой, потому что не знает пароль.

В первом случае, проблему можно решить, если добавить еще проверку по username и обновлять данные. Но username также можно скорректировать. В общем и целом, это должен быть id пользователя, потому что он уже не поменяется. Этого поля не хватает.
Во втором случае, можно редиректить на страницу с изменением пароля, но это плохой клиентский путь, потому что клиенту нужно догадаться зачем ему это нужно сделать. Следовательно, нужно добавить какое-то описание.

[fey]

@d1z3d а как на hexlet-cv, code-basics работает?

[d1z3d]

@d1z3d а как на hexlet-cv, code-basics работает?

Сравнивал как раз с hexlet-cv. У меня нет возможности скорректировать email.

[fey]

@d1z3d а можете посмотреть. как в самом Хекслете? ru.hexlet.io

[fey]

@d1z3d Посмотрите, предоставляет ли Github какие-то дополнительные данные, которые нельзя подменить?
https://docs.github.com/en/rest/users/users?apiVersion=2022-11-28#get-the-authenticated-user - например. Там есть ID. Давайте записывать ID пользователя.

{
  "login": "fey",
  "id": 697178,
  "node_id": "MDQ6VXNlcjY5NzE3OA==",
  "avatar_url": "https://avatars.githubusercontent.com/u/697178?v=4",
  "gravatar_id": "",
  "url": "https://api.github.com/users/fey",
  "html_url": "https://github.com/fey",
  "followers_url": "https://api.github.com/users/fey/followers",
  "following_url": "https://api.github.com/users/fey/following{/other_user}",
  "gists_url": "https://api.github.com/users/fey/gists{/gist_id}",
  "starred_url": "https://api.github.com/users/fey/starred{/owner}{/repo}",
  "subscriptions_url": "https://api.github.com/users/fey/subscriptions",
  "organizations_url": "https://api.github.com/users/fey/orgs",
  "repos_url": "https://api.github.com/users/fey/repos",
  "events_url": "https://api.github.com/users/fey/events{/privacy}",
  "received_events_url": "https://api.github.com/users/fey/received_events",
  "type": "User",
  "site_admin": false,
  "name": "Nikolay Gagarinov",
  "company": "@Hexlet",
  "blog": "https://go.redav.online/22dba39c7f71c491",
  "location": null,
  "email": null,
  "hireable": null,
  "bio": "У тебя не будет багов, если ты не пишешь код.",
  "twitter_username": null,
:

[fey]

@d1z3d Я поспрашивал у разработчиков Хекслета:

в случае когда акки связаны аутентификация происходит не по емейлам, а по самому акку.
емейл с гитхаба берётся только если на хекслете он не найден. В таком случае на хекслете будет зарегистрированн новый акк с данными с гитхаб

[d1z3d]

Про то, что не хватает id пользователя - я имел в виду, что его не хватает в таблице бд для его сохранения. Из гитхаба он приходит.

[d1z3d]

@d1z3d Я поспрашивал у разработчиков Хекслета:

в случае когда акки связаны аутентификация происходит не по емейлам, а по самому акку.

емейл с гитхаба берётся только если на хекслете он не найден. В таком случае на хекслете будет зарегистрированн новый акк с данными с гитхаб

Ок, теперь мне нужно это осознать)

[fey]

@d1z3d я себе так это представляю)
имеем таблицу аккаунтов, которая связана с юзерами. Когда гость жмет на вход через Github, то проверяем, есть ли связанный аккаунт с пользователем. Если нет, то создаем пользователя + аккаунт с этой привязкой.
Если у нас пользователь уже существует, то создаем аккаунт + привязываем пользователя к нему (проверяем по емейлу).

[d1z3d]

@d1z3d я себе так это представляю) имеем таблицу аккаунтов, которая связана с юзерами. Когда гость жмет на вход через Github, то проверяем, есть ли связанный аккаунт с пользователем. Если нет, то создаем пользователя + аккаунт с этой привязкой. Если у нас пользователь уже существует, то создаем аккаунт + привязываем пользователя к нему (проверяем по емейлу).

Ок. Продолжу работу после 23 сентября.

[d1z3d]

Продолжил дальше заниматься задачей

[d1z3d]

@fey, добрый день!
Добавил создание таблицы, проверку пользователя по ID из Github-а, обновление email (с учетом констрейта), username (с учетом констрейта), фамилию и имени