Hexlet / hexlet-correction

Typo Reporter
https://fixit.hexlet.io/
GNU Affero General Public License v3.0
51 stars 84 forks source link

[#80] GitHub authorization #285

Open d1z3d opened 3 months ago

d1z3d commented 3 months ago

@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 commented 3 months ago

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

d1z3d commented 3 months ago

@fey I redeployed a last commit. Now it works image

fey commented 3 months ago

Still cant enter to deployed demo

fey commented 3 months ago

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

d1z3d commented 3 months ago

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

fey commented 3 months ago

@d1z3d cant load demo :C

Please add inctruction, we ll check auth on local

fey commented 3 months ago

@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. image image

d1z3d commented 3 months ago

@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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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

d1z3d commented 3 months ago

@Malcom1986 hey I will add tests later. I don't why but nobody can't open render. I deploy few times. Can you check locally as well as fey?

fey commented 3 months ago

@d1z3d i checked locally. Can you add link on signup page? Like it hexlet-cv image

d1z3d commented 3 months ago

@d1z3d i checked locally. Can you add link on signup page? Like it hexlet-cv

image

Okey

d1z3d commented 3 months ago

@d1z3d i checked locally. Can you add link on signup page? Like it hexlet-cv image

Okey

Done

d1z3d commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

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

Malcom1986 commented 3 months ago

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

fey commented 3 months ago

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

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

d1z3d commented 3 months ago

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

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

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

d1z3d commented 2 months ago

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

Malcom1986 commented 2 months ago

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

Malcom1986 commented 2 months ago

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

d1z3d commented 2 months ago

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

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

d1z3d commented 2 months ago

@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 commented 2 months ago

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

d1z3d commented 2 months ago

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

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

fey commented 2 months ago

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

fey commented 2 months ago

@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 commented 2 months ago

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

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

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

d1z3d commented 2 months ago

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


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

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

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

fey commented 2 months ago

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

d1z3d commented 2 months ago

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

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

d1z3d commented 1 month ago

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

d1z3d commented 1 month ago

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