Skip to content

Conversation

gopla
Copy link
Collaborator

@gopla gopla commented Aug 23, 2020

Ini kalau mau cek diclone dulu ya,
Soalnya link callback di googlenya cuma aku set di local sama production

nanti di file gOauth.ts balik url local ke index ke-0

Untuk testing google oauthnya sendiri aku belum nemu gimana caranya :''

@SultanKs4 SultanKs4 temporarily deployed to api-blueprint-pr-11 August 23, 2020 07:25 Inactive
@gopla gopla requested review from debbysa and SultanKs4 and removed request for debbysa August 23, 2020 07:26
@debbysa
Copy link
Collaborator

debbysa commented Aug 25, 2020

ukay, aku coba dulu ya

Comment on lines +130 to +136
if (!isUserExist) {
const userReq: UserRequest = {
email: data.email,
password: 'blueprint123',
}
isUserExist = await this.createUser(userReq)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

apakah kalo gini nanti misal kita login tanpa klik login with google (tapi sudah sign up with google dulu) itu asal tahu email dan masukin password blueprint123 langsung bisa masuk?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@SultanKs4 iya betul, menurutku harusnya ketika signup by google, data baru yang harus disimpan ke schema user ini hanya email saja. Jadi ketika user sudah singup by google dan login menggunakan form, dia gak bakal bisa melakukannya karena passwordnya gak ada. Jadi harus login by google, terus ngeset passwordnya setelah login (fitur ini belum ada, bisa ditambahin nanti)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

jadi di user modelnya untuk password requirednya dihapus?

Copy link
Collaborator

Choose a reason for hiding this comment

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

biasanya kan gini kalo kasus aku nyobak di trello gitu,
kalo sign up by email, dia milih emailnya yang mana. terus masukkan password email.
habis itu setelah sign up dia langsung bisa masuk ke trello nya.

terus kalo mau login by google, mekanismenya sama kyk sign up. milih email sama masukin password email.

Copy link
Collaborator

Choose a reason for hiding this comment

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

kalo dia mau login biasa, harusnya masukin username sama password emailnya dia. jdi password ini tetep keisi

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gopla tapi kalo requirednya dihapus nanti yang login pake email biasa bisa daftar tanpa password brarti?

@debbysa kalo login biasa masukin username sama password email itu pas daftar kita tahu password emailnya pendaftar buat dimasukin ke database gimana mbak?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@SultanKs4 Bukannya udah dipasang sanitizer ya disana?
Jadi kalau mau daftar biasa tetep masukin password

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gopla lupa sanitizer nya wkkw iya bisa sih harus nya

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gopla iyups, jadi kalo waktu sign up by gmail, itu kan dapet email, username, & password gmail. Nah itu di save ke db

Copy link
Collaborator

Choose a reason for hiding this comment

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

@debbysa kayanya untuk password ga dapet deh soalnya kan itu secret banget, aku cari di OAuth 2.0 Scope juga ga ada OAuth 2.0 Scope

@SultanKs4 SultanKs4 temporarily deployed to api-blueprint-pr-11 August 31, 2020 04:57 Inactive
@gopla
Copy link
Collaborator Author

gopla commented Aug 31, 2020

Ini aku cuma pindah ke env seperti saran @SultanKs4

Untuk masalah passwordnya masih belum ngeh ._,

Comment on lines +102 to +105
const userToken = jwt.sign(
token as Credentials,
process.env.JWT_SECRET as string
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

ini jadinya nanti ada 2 token ya. Satu dari credentials login google, satunya dari email.
Credentials ini otomatis dibuatkan sama googlenya?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Iya,
tapi tetep kita pakenya yang token buatan sendiri

Token dari google cuma buat retrieve data dari email kita

Comment on lines +138 to +142
const token: any = await jwt.sign(
{ email: isUserExist.email },
process.env.JWT_SECRET || ''
)
resolve(token)
Copy link
Collaborator

Choose a reason for hiding this comment

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

aku masih bingung baris ini sih.
di fungsi authenticateUser() sebelumnya, aku sudah buat jwt.sign.
Disini kenapa dikasih jwt.sign lagi?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Karena di fungsi authenticateUser() kan ada cek passwordnya

Sedangkan kalau login by google kita ga perlu masukin password
Jadi aku buat jwt.sign lagi disini

Copy link
Collaborator

Choose a reason for hiding this comment

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

@gopla jadi ini jwt sign nya beda lagi sama yang sebelumnya ya? yang token as credentials

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@debbysa iya, beda. karena token as credentials yang atas itu bener udah contain data kita tapi masih dalam bentuk enkripsi

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants