Skip to content
This repository was archived by the owner on Jul 23, 2025. It is now read-only.

[#320] add registration and authorization with GitHub#322

Closed
ean3ena wants to merge 7 commits intoHexlet:mainfrom
ean3ena:320-github-oauth2
Closed

[#320] add registration and authorization with GitHub#322
ean3ena wants to merge 7 commits intoHexlet:mainfrom
ean3ena:320-github-oauth2

Conversation

@ean3ena
Copy link
Copy Markdown

@ean3ena ean3ena commented May 29, 2025

No description provided.

@fey
Copy link
Copy Markdown
Collaborator

fey commented May 30, 2025

@ean3ena Привет, этот ПР можно смотреть?

@ean3ena
Copy link
Copy Markdown
Author

ean3ena commented May 30, 2025

Привет! Да, я его отправил на рассмотрение.

@fey
Copy link
Copy Markdown
Collaborator

fey commented May 30, 2025

@ean3ena тогда задеплой, пожалуйста, демку на render.com, чтобы кто-то мог ее посмотреть-потыкать кнопки.

@fey
Copy link
Copy Markdown
Collaborator

fey commented May 30, 2025

И плюсом, в ридми добавить инструкцию по настройке аутентификации через Github

@ean3ena
Copy link
Copy Markdown
Author

ean3ena commented May 30, 2025

Привет! Добавил в ридми и задеплоил https://hexlet-correction-8nal.onrender.com

Copy link
Copy Markdown
Collaborator

@fey fey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кнопки потыкал, работает. По коду остальному лучше чтобы кто-то еще поревьювил.

Вопрос - а как переменные окружения установить локально, при разработке?

Плюс, видел, что в тг ругались по поводу nickname/name? В итоге что решили? На форме вообще Nickname нет, а есть username

)
.addFilterBefore(corsFilter(dynamicCorsConfigurationSource), CorsFilter.class);

if (oauth2Enable) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isOAuth2Enabled, тк это состояние/флаг

@fey
Copy link
Copy Markdown
Collaborator

fey commented Jun 2, 2025

@Sanapol Сделай ревью, пожалуйста.

@fey
Copy link
Copy Markdown
Collaborator

fey commented Jun 2, 2025

@ean3ena подскажи, тесты есть на новую фичу? Вижу изменения в SignupControllerIT, но не вижу, что это на oauth2 🤔

Если нет, то сразу тесты давай делать.

@ean3ena
Copy link
Copy Markdown
Author

ean3ena commented Jun 2, 2025

Вопрос - а как переменные окружения установить локально, при разработке?

Любым известным способом, я создал файл .env в корне проекта

@ean3ena
Copy link
Copy Markdown
Author

ean3ena commented Jun 2, 2025

Если нет, то сразу тесты давай делать.

Не делал, буду думать как реализовать

@fey
Copy link
Copy Markdown
Collaborator

fey commented Jun 2, 2025

Если нет, то сразу тесты давай делать.

Не делал, буду думать как реализовать

мб что-то из этого ПРа поможет - https://github.com/Hexlet/hexlet-correction/pull/285/files#diff-f3562bf289ab10b865a87fcc11c5b79bd25d4805795cb5a514c417614ba29117

И пример на PHP - https://github.com/Hexlet/hexlet-sicp/blob/main/tests/Feature/Http/Controllers/Auth/Social/GithubControllerTest.php

@fey
Copy link
Copy Markdown
Collaborator

fey commented Jun 2, 2025

Вопрос - а как переменные окружения установить локально, при разработке?

Любым известным способом, я создал файл .env в корне проекта

Тогда в ридми это стоит добавить. Плюсом создать файл-пример, в котором будут заданные переменные. Этот файл будет использоваться для стартовой настройки проекта локально.

@ean3ena
Copy link
Copy Markdown
Author

ean3ena commented Jun 2, 2025

Плюсом создать файл-пример, в котором будут заданные переменные.

Мне про все способы добавления локальных переменных писать?

@ean3ena
Copy link
Copy Markdown
Author

ean3ena commented Jun 2, 2025

Плюсом создать файл-пример, в котором будут заданные переменные.

А где этот файл должен быть в структуре проекта? Файл .env заигнорен в гит, он не выгружается на GitHub, я в описании в выделенном блоке показал как в нем прописывать переменные и значения.

@fey
Copy link
Copy Markdown
Collaborator

fey commented Jun 2, 2025

Плюсом создать файл-пример, в котором будут заданные переменные.

А где этот файл должен быть в структуре проекта? Файл .env заигнорен в гит, он не выгружается на GitHub, я в описании в выделенном блоке показал как в нем прописывать переменные и значения.

Смотря откуда он должен подгружаться. Если в корне, то туда положить example.

Все способы не нужно. Нужно прописать кейсы, когда локально разворачивают проект. По сути .env.example будет являться частью документации проекта (какие переменные он использует).

Т.е. в идеале, если я поднимаю проект локально, я должен минимум движений делать. ПредставЬ, что проект поднимает тестировщик по инструкции.

@Sanapol
Copy link
Copy Markdown
Contributor

Sanapol commented Jun 2, 2025

@fey вот твое мнение нужно, я считаю, что nickname более читабелен и чтобы его не менять я создал: CustomOAuth2User extends DefaultOAuth2User

Андрей считает что легче заменить nickname на name, чем писать кастомный класс, @ean3ena поправь если я не правильно твои слова передал

так вот вопрос как будет лучше?

@fey
Copy link
Copy Markdown
Collaborator

fey commented Jun 3, 2025

@fey вот твое мнение нужно, я считаю, что nickname более читабелен и чтобы его не менять я создал: CustomOAuth2User extends DefaultOAuth2User

Андрей считает что легче заменить nickname на name, чем писать кастомный класс, @ean3ena поправь если я не правильно твои слова передал

так вот вопрос как будет лучше?

Подскажи, nickname - этот термин он использоваться где будет. Я посмотрел по ПРам и вижу несогласованный нейминг) Github, Yandex предоставляют Login, поверх которого используется метод getUsername, далее username и используется как термин.

Впервые nickname я так понял появился тут - https://github.com/Hexlet/hexlet-correction/pull/311/files

мне кажется, надо договориться об общей терминологии и поправить то, что есть.

На формах используется username, я бы так и назвал везде это поле/атрибут, т.е. это уникальное имя пользователя/идентификатор, хотя вход у нас щас реализован по емейлу.

Вот. Я думаю нейминг со стороны oauth2 не так важен, ведь мы потом смапим данные в наши сущности. 🤔

Как мысленный эксперимент - нарисуй несколько кругов-слоев приложения. И ближе к внешнему кругу у тебя будет oauth2 и вот на каждом слое у тебя должна быть одна терминология.

К примеру в Hexlet SICP у модели пользователя есть имя - оно не уникальное, и просто отображается как строка. Но если мы регаемся через Github, то берем сперва никнейм, а потом имя, если ника нет.

https://github.com/search?q=repo%3Alaravel%2Fsocialite%20nickname&type=code

Ну и вот пример, Не знаю, видно ли. Либа, которую используем в SICP - она работает оберткой над провайдерами и конвертирует к имени атрибута логин/никнейм/юзернейм.

В общем, много написал :D

  1. В ядре приложения (модели и тд), username - норм имя атрибута/поля для уник. имени пользователя
  2. Nickname - норм, если приходит от либы-провайдера oauth2
  3. Name - больше отражает имя пользователя, но ближе к тому, что может быть не уникальным.

@fey
Copy link
Copy Markdown
Collaborator

fey commented Jun 25, 2025

смержено в 7634a94

@fey fey closed this Jun 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Feature request: Реализовать регистрацию/авторизация: GitHub

3 participants