-
Notifications
You must be signed in to change notification settings - Fork 84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: autenticação via Github #89
Conversation
apps/server/src/app.controller.ts
Outdated
async getUsers() { | ||
const users = await this.prisma.user.findMany() | ||
@Post() | ||
async getUser(@Req() request: Request, @Res() response: Response) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
acho que tá um pouquinho estranho esse @Post()
e o nome da função começa com get
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boa, eu só editei o método que tinha pra testar a requisição do mobile e acabei deixando o nome antigo 😅
@vinifraga brabo demais! resolve os conflitos pra nós? |
@@ -1,3 +1,4 @@ | |||
import 'dotenv/config' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No Nest acredito que não precisa disso se usarmos o ConfigModule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Se não realizar esse import
, o clerk
não entende a váriavel ambiente CLERK_SECRET_KEY
.
apps/mobile/App.tsx
Outdated
async function handleVerifySession() { | ||
try { | ||
const { sessionId, token } = await getSessionInfo() | ||
const response = await axios.post('http://192.168.15.4:3333', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acho legal colocar a url via env, não?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Esse código no mobile é apenas para demonstração e teste da funcionalidade aqui na PR, essa página não existe no figma. Vou apagar esse código ao final, mas posso deixar um service
com uma env
para o IP.
apps/mobile/App.tsx
Outdated
token, | ||
}) | ||
|
||
console.log(response.data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aqui sobrou um console log
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Esse código no mobile é apenas para demonstração e teste da funcionalidade aqui na PR, essa página não existe no figma.
const sessionId = request.body.sessionId | ||
const token = request.body.token | ||
|
||
if (typeof sessionId === 'string') { | ||
const session = await sessions.verifySession(sessionId, token) | ||
|
||
if (!session) { | ||
return response.status(400).json({ | ||
error: 'Session not verified', | ||
}) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Acho que todo esse bloco de código poderia ser substituído pelo auth guard criado por @eletroswing na PR #94
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Boa, esse código no back é apenas para demonstração e teste da funcionalidade aqui na PR, toda melhora é bem vinda mas como não trabalho muito com NestJS vou deixar essa parte para outra pessoa.
Opa, por aqui na PR disse que não tenho acesso. Tem alguma sugestão? |
@vinifraga você precisa resolver os conflitos localmente, isto é, combinar o código mais atualizado do projeto com o código que você fez. Isso pode ser feito utilizando rebase ou dando um "git pull origin main" na sua branch e passando por cada conflito e resolvendo. Se quiser ajuda, me chama Discord. |
Bom dia pessoal, encontrei esse artigo trazendo atualizações do Clerk para o Expo: https://clerk.com/blog/changelog-2023-04-07?utm_source=changelog&utm_medium=email_campaign Vou testar esse novo hook para ver se funciona bem e resolve o problema do Login social no iOS. |
@vinifraga Vou aguardar você testar isso antes de dar merge. |
Essas últimas alterações foram implementar algumas mudanças pontuais:
|
📋 Descrição
Bom dia pessoal, trago uma proposta de autenticação com o Clerk principalmente para o mobile, mas também sugestões para o backend. Vou dividir essa issue em duas seções: mobile e back.
Mobile
Libs utilizadas:
scheme
.Docs utilizadas:
Proposta
Utilizarmos o contexto AuthContext para gerenciar todo o fluxo que precisarmos de autenticação, deixando "transparente" para o restante dos devs as particularidades do Clerk. Esse contexto disponibiliza as seguintes funções e variáveis:
provider
escolhido. Por enquanto, utilizamos apenas o GitHub. Além disso, ela também automaticamente gerencia se o usuário precisa ser cadastrado (primeiro acesso) ou logado;true
caso alguma das três funções anteriores estiverem sendo executadas.Para consumir os dados desse contexto, criei um
hook
chamado useAuth (temos que tomar cuidado na importação pois o@clerk/clerk-expo
exporta um hook de mesmo nome)Para centralizar os providers em um ambiente, sugiro a estrutura do AppProvider
Apesar do Clerk não disponibilizar componentes visuais no Expo, sugiro utilizarmos os
helpers
SignedIn e SignedOut para exibir condicionalmente componentes dependendo do status da autenticação do usuário.Problemas
Apesar da documentação do Clerk acusar que a SDK deles não é compatível com a SDK 48 do Expo, não encontrei nenhum problema com a lib deles. Porém, definitivamente o maior empecilho dessa PR foi a incompatibilidade da SDK 48 do Expo e a lib
expo-auth-session
no Expo Go iOS. Tentei de várias formas resolver esse problema mas não consegui. Então quem for tentar autenticar no Expo Go viaiOS
não funciona por enquanto. Existe uma gambiara caso você realmente precise autenticar via iOS:npx expo run:ios
Backend
Libs utilizadas:
Docs utilizadas:
Proposta
Aqui não me aprofundei muito pessoal, basicamente importei os métodos
sessions
eusers
da SDK para validar a sessão e buscar os dados do usuário, respectivamente. Então basicamente editei a rota noAppController.tsx
para receber os parâmetrossessionId
etoken
que o mobile envia, validei a sessão, obtive o id do usuário e o utilizei para buscar o restante das informações.Problemas
Como a SDK do Clerk já retorna uma instância inicializada e populada automaticamente com a variável ambiente
CLERK_SECRET_KEY
, eu tive que realizar oimport 'dotenv/config'
. Além disso, como não temos umarecipe
do NestJS para o Clerk e como o Clerk retorna uma função em vez de uma classe, tive dificuldade em criar um provider como fizemos com o Prisma.Fixes #61
🛠️ Tipo da mudança
Exclua as opções que não são relevantes.
🧪 Como isso foi testado?
Não adicionei testes para essa parte pois ainda precisamos definir o setup de testes no mobile.
✅ Checklist:
<type>(scope): subject
. Por exemplo:feat(mobile): add new feature