Skip to content
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: use oauth for login #29

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open

feat: use oauth for login #29

wants to merge 26 commits into from

Conversation

anbraten
Copy link
Contributor

@anbraten anbraten commented Nov 26, 2024

closes #28

Basic login support by entering handle / did. It uses @atcute/oauth-browser-client and @atcute/client for now. The profile info is loaded after login.

It's not perfect yet and some parts have been ignored for now to keep changes in this PR to a minimum. Thereby next tasks would be:

  • replace atproto types and atcute imports with tsky wrappers
  • cleanup composables/users.ts and remove mastodon related helpers

Copy link

netlify bot commented Nov 26, 2024

Deploy Preview for nimbus-docs canceled.

Name Link
🔨 Latest commit 2c19a2d
🔍 Latest deploy log https://app.netlify.com/sites/nimbus-docs/deploys/674d8b0e600a4c0008387cdc

Copy link

netlify bot commented Nov 26, 2024

Deploy Preview for nimbus-town ready!

Name Link
🔨 Latest commit 2c19a2d
🔍 Latest deploy log https://app.netlify.com/sites/nimbus-town/deploys/674d8b0e7abdd300088cf51f
😎 Deploy Preview https://deploy-preview-29--nimbus-town.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@IonianPlayboy IonianPlayboy added enhancement New feature or request dependencies Pull requests that update a dependency file bluesky parity Feature needed to reach parity with Bluesky p3-significant High priority enhancement (priority) feat: settings Auth and configuration and removed enhancement New feature or request labels Nov 26, 2024
@IonianPlayboy
Copy link
Collaborator

The CI is failing because of a pnpm error, @anbraten can you check if you have installed the new dependency with the right command ? (You can check the contributing guide to see how it should be done.)

@anbraten

This comment was marked as resolved.

pnpm-lock.yaml Outdated Show resolved Hide resolved
@anbraten anbraten marked this pull request as ready for review November 28, 2024 00:36
@@ -15,7 +15,7 @@ if (import.meta.server && !route.path.startsWith('/settings')) {
}

// We want to trigger rerendering the page when account changes
const key = computed(() => `${currentUser.value?.server ?? currentServer.value}:${currentUser.value?.account.id || ''}`)
const key = computed(() => currentUserDid.value)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review note: did is the globally unique id of a bsyk account.

@@ -1,5 +1,5 @@
<script setup lang="ts">
const { busy, oauth, singleInstanceServer } = useSignIn()
const { busy, signIn, singleInstanceServer } = useSignIn()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review note: renamed oauth to signIn to not get confused with oauth-callback

const input = server.value.trim()
if (input.startsWith('https://'))
server.value = input.replace('https://', '')
const input = handle.value.trim()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review note: removed quite some code here as you don't need to select a server with bsky as there is an integrated discovery system for a handle / did.

import type { mastodon } from 'masto'

export function getDisplayName(account: mastodon.v1.Account, options?: { rich?: boolean }) {
// TODO: remove once mastondon support was replaced
export type Account = mastodon.v1.Account | ProfileViewDetailed
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review note: to avoid too many changes I changed most of these functions to accept bsky and mastodon accounts for now.

if (fromServer !== server)
loginTo(masto, { server })
if (fromServer !== server) {
// loginTo(masto, did) // TODO: do we need to change accounts for links?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review note: This needs some further investigation later on as we don't know the server of a post we would always use the currently active did. Only issue might be that only another did which was used before has access to see a post / etc. Not sure how we would know this.

Comment on lines +35 to +37
"@atcute/client": "^2.0.6",
"@atcute/oauth-browser-client": "^1.0.7",
"@atproto/api": "^0.13.18",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review note: For now using atcute lib for api calls and oauth and @atproto for types til the tsky lib exports them.

@@ -139,9 +142,9 @@
"sharp-ico": "^0.1.5",
"simple-git-hooks": "^2.11.1",
"tsx": "^4.19.2",
"typescript": "^5.4.4",
"typescript": "5.6.2",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review note: Had to be pinned to avoid an error with vue-tsc

Comment on lines +27 to +34
<template v-if="currentUser">
<p>Hello {{ currentUser?.profile.handle }}!</p>
<p>Here you will see your timeline later on</p>
</template>
<template v-else>
<p>You are currently not authenticated.</p>
<p>Here you will see a public timeline later on</p>
</template>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review note: for now only showing the currently active handle. In the next step this should be replace with a bsky feed.

Comment on lines +44 to +48
// eslint-disable-next-line no-console
console.log('todo, do we need to reload?')
// setTimeout(() => {
// window.location.reload()
// }, 0)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Review note: as we do reload vue components in app.vue with a computed on the current handle already this should probably be not necessary. Currently keeping it so we could debug with the console statement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bluesky parity Feature needed to reach parity with Bluesky dependencies Pull requests that update a dependency file feat: settings Auth and configuration p3-significant High priority enhancement (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Login via OAuth
4 participants