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

MiAuthのチェックは /api/miauth/{session}/check ではなく /miauth/{session}/check にしたい #6418

Open
YuzuRyo61 opened this issue May 29, 2020 · 11 comments · Fixed by #6442
Labels
🧩API Interface between server and client 🐛Bug Unexpected behavior ✨Feature This adds/improves/enhances a feature

Comments

@YuzuRyo61
Copy link
Contributor

💡 Summary

MiAuthのチェックURLにPOSTすると404になる

🙂 Expected Behavior

セッションIDが有効かつ認証許可した場合、HTTP 200でjson形式のレスポンスが返ってくる

☹️ Actual Behavior

認証許可してもHTTP 404が出る

📝 Steps to Reproduce

  1. miauthのセッションIDを作り、ユーザー認証を行う(iconやcallbackは未指定。)
  2. /miauth/{セッションID}/check にPOSTする

📌 Environment

Misskey v12.38.1

@YuzuRyo61 YuzuRyo61 added the ⚠️bug? This might be a bug label May 29, 2020
@YuzuRyo61
Copy link
Contributor Author

/api/miauth/{セッションID}/check にPOSTしたら正常に返ってきました。

Misskey APIのドキュメントに問題があるようです。

ユーザーが連携を許可した後、https://misskey.io/miauth/{session}/checkにPOSTリクエストすると、レスポンスとしてアクセストークンを含むJSONが返ります。

もし改善できるのであれば、ドキュメントに揃えていただきたいです。

@tamaina
Copy link
Member

tamaina commented May 29, 2020

404が返ってきておかしいなとは思っていたけど、そういう仕様かと思ってた

@YuzuRyo61
Copy link
Contributor Author

404が返ってきておかしいなとは思っていたけど、そういう仕様かと思ってた

/miauth/{session}はウェブクライアント側のルーターで処理しているようですが、
/miauth/{session}/checkはサーバー側のルーターで処理しているようなので多分サーバー側のルーターのプレフィックスが影響しているのかもしれません。

@tamaina
Copy link
Member

tamaina commented May 29, 2020

/apiのほうを正規のurlにして/miauth/.../checkに来たら301って感じの対応でどうでしょう?

@tamaina tamaina added 🐛Bug Unexpected behavior 🧩API Interface between server and client and removed ⚠️bug? This might be a bug labels May 29, 2020
@mei23
Copy link
Contributor

mei23 commented May 29, 2020

POSTを301するとGETで飛んじゃいます

@YuzuRyo61
Copy link
Contributor Author

/apiのほうを正規のurlにして/miauth/.../checkに来たら301って感じの対応でどうでしょう?

とにかく、miauthのURLは統一すべきです。

@tamaina
Copy link
Member

tamaina commented May 29, 2020

api統一でいいか

@tamaina
Copy link
Member

tamaina commented May 29, 2020

ドキュメントを書き換えるだけにする

@YuzuRyo61
Copy link
Contributor Author

router.post('/miauth/:session/check', async ctx => {
	const token = await AccessTokens.findOne({
		session: ctx.params.session
	});

	if (token && !token.fetched) {
		AccessTokens.update(token.id, {
			fetched: true
		});

		ctx.body = {
			ok: true,
			token: token.token,
			user: await Users.pack(token.userId, null, { detail: true })
		};
	} else {
		ctx.body = {
			ok: false,
		};
	}
});

上記のスクリプトは今 src/server/api/index.ts上に書いてあるみたいですが、これをsrc/server/index.tsに移動すればいいのではないでしょうか。

@syuilo syuilo reopened this Jun 4, 2020
@syuilo
Copy link
Member

syuilo commented Jun 4, 2020

/api/miauth/{session}/check ではなく /miauth/{session}/check を意図しているのでそのように修正します

syuilo added a commit that referenced this issue Jun 4, 2020
syuilo added a commit that referenced this issue Jun 4, 2020
tamaina added a commit to tamaina/misskey that referenced this issue Jun 4, 2020
tamaina added a commit to tamaina/misskey that referenced this issue Jun 4, 2020
mei23 added a commit to mei23/misskey that referenced this issue Jun 4, 2020
syuilo added a commit that referenced this issue Jun 4, 2020
@tamaina tamaina changed the title MiAuthのレスポンスが返ってこない MiAuthのチェックは /api/miauth/{session}/check ではなく /miauth/{session}/check にしたい Jun 11, 2020
@tamaina tamaina reopened this Jun 11, 2020
@tamaina tamaina removed the 🧩API Interface between server and client label Jun 11, 2020
@tamaina tamaina added ✨Feature This adds/improves/enhances a feature 🧩API Interface between server and client labels Jun 11, 2020
@tamaina
Copy link
Member

tamaina commented Jun 11, 2020

Note: 経過説明

  1. 83ec906 をコミットした
  2. 83ec906 だとリモートからの投稿を受信できないことに12.39.0リリース直前に気づく リモートからの投稿を受信できていない #6444
  • 結局バグを孕んだまま12.39.0がリリースされてしまった
  1. 83ec906Revertし、とりあえず/api/...なURLを正規のものとした(12.39.1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧩API Interface between server and client 🐛Bug Unexpected behavior ✨Feature This adds/improves/enhances a feature
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

4 participants