Deployments that only allow third-party sign-up can now disable local
registration end to end: the register endpoint returns 403 when
skillhub.auth.local.registration-enabled is false, and the web UI hides
the entry when SKILLHUB_WEB_REGISTRATION_ENABLED is false. Also log
OAuth2 login failures so callback errors are diagnosable from server
logs. Fixes the runtime-config entrypoint so registration defaults are
actually exported to envsubst.
Signed-off-by: yhd <yhd4711499@live.com>
Higress terminates TLS and forwards plain HTTP without a usable
X-Forwarded-Proto, so {baseUrl} expansion produced http:// OAuth2
redirect URIs and Secure session cookies were dropped. Rewrite the
scheme to https when the request host matches skillhub.public.base-url.
Signed-off-by: yhd <yhd4711499@live.com>
Addresses review on PR #696:
1. The authorize step now uses the currently documented endpoint
https://accounts.feishu.cn/open-apis/authen/v1/authorize with standard
OAuth2 parameters (client_id, optional scope), so the legacy
app_id/no-scope URI rewrite in SkillHubOAuth2AuthorizationRequestResolver
is removed. The authorize host is overridable via
OAUTH2_FEISHU_AUTHORIZE_URI; token and userinfo endpoints stay on
OAUTH2_FEISHU_BASE_URI.
2. FeishuClaimsExtractor no longer claims emailVerified: Feishu emails are
admin-imported and carry no real-time verification signal, so
emailVerified is always false. EMAIL_DOMAIN policy matches the email
domain only and is unaffected.
Signed-off-by: yhd <yhd4711499@live.com>
Feishu deviates from standard OAuth2: the authorize endpoint requires
app_id instead of client_id, userinfo returns a {code, msg, data}
envelope with errors reported as HTTP 200, and the token endpoint uses
client_secret_post. Reuse the Spring oauth2Login pipeline and override
only the userinfo loading step via a ProviderOAuth2UserService extension
point, keeping GitHub/GitLab behavior unchanged. Bindings use open_id
as subject; union_id is kept in extra for future cross-app migration.
Signed-off-by: yhd <yhd4711499@live.com>
Downloading a skill whose slug is non-ASCII (e.g. a Chinese name) through
the ClawHub CLI compatibility route failed: the 302 Location header was
built by string-concatenating the raw slug, and Tomcat encodes header
values as ISO-8859-1, so a character outside 0-255 makes it drop the
Location header entirely and the download breaks. The skillhub CLI path
was unaffected because it doesn't go through this redirect.
Build the Location with UriComponentsBuilder.pathSegment(...).encode(), so
each segment is percent-encoded while the '/' separators stay literal.
"需求" becomes %E9%9C%80%E6%B1%82 and the header is ISO-8859-1-writable.
Fixes#658
Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
Remove terminal review tasks before deleting an allowed skill version. Lock all versions of the aggregate in stable order so concurrent deletes preserve the last-version invariant and return business errors instead of 500 responses.
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
* fix(api): tell callers why a request was forbidden
The scope filter already computes an exact reason ("Missing API token
scope: skill:delete", "API token cannot access endpoint: /x") and the
access-denied handler discarded it, returning a bare "Forbidden" for
every case: missing scope, endpoint closed to API tokens, and paths
that simply don't exist. Clients cannot tell those apart, so they
guess — the published CLI reports every 403 as "token may lack
required scope", which sent us debugging token scopes for an hour when
the real causes were a revoked token and a mistyped namespace path.
The reason now rides in the response via a new error.forbidden.detail
message (en + zh), and is logged alongside the exception type.
Signed-off-by: Gal Eyal <gal.e@popai.health>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(api): safely expose API token denial reasons
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
---------
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
The shared RedisTemplate uses GenericJackson2JsonRedisSerializer with
the application ObjectMapper, which embeds no type information, so
stored DeviceCodeData deserializes as a LinkedHashMap. The typed casts
in pollToken and authorizeDeviceCode then throw ClassCastException on
every call, making the whole device authorization flow unusable
(every poll returns 500).
Convert the raw value with ObjectMapper.convertValue instead of
casting; this reads both the current untyped map format and any typed
format, so no stored-data migration is needed. Adds bean setters to
DeviceCodeData for map conversion and regression tests that feed the
service exactly what Redis returns in production (untyped maps).
Fixes#604
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Gal Eyal <gal.e@popai.health>
* fix(publish): delete review tasks of any status when replacing a version
Re-uploading a rejected version under the same version number returned
HTTP 500. deleteReplaceableVersionArtifacts only removed a PENDING review
task, but a rejected version owns a REJECTED one; that row kept a foreign
key on the skill_version, so the subsequent delete hit a constraint
violation that surfaced as a 500.
Delete every review task attached to the version instead.
Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
* test(publish): drop the spring-test dependency from the new test
skillhub-domain has no spring-test on its test classpath, so
ReflectionTestUtils does not resolve there. Use plain JDK reflection for
setting the generated id and invoking the private method.
Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
* fix(publish): constrain rejected version replacement
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
* test(publish): verify replaced review is deleted
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
* test(e2e): use generated API response types
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
---------
Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
Co-authored-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
The @Async rebuildSkill fix relied on a fresh thread giving a clean
transaction boundary. But skillhubEventExecutor uses CallerRunsPolicy:
under saturation the rejected task runs on the caller (request) thread,
back inside the afterCommit synchronization phase — the original failure
context where the @Transactional index write is silently dropped.
Mark SearchIndexService.index as REQUIRES_NEW so it always suspends any
lingering post-commit synchronization and commits in its own transaction,
independent of whether the async dispatch actually happened.
Add regression tests: detach removes the label keyword, and a synchronous
rebuild inside the afterCommit phase still persists the document (fails
without REQUIRES_NEW).
Signed-off-by: shychee <shychee96@gmail.com>
Attaching or detaching a skill label triggers a search index rebuild via
an afterCommit callback. Because LabelSearchSyncService.rebuildSkill ran
synchronously on the request thread, the @Transactional index write
executed inside the already-committed transaction-synchronization phase
and was silently dropped -- the search document was never written, so
label keywords never became searchable.
Move rebuildSkill onto the skillhubEventExecutor with @Async (matching the
existing rebuildSkills batch path) so the rebuild runs on a fresh thread
and transaction. Add an integration test that fails on the old synchronous
path and passes with the async fix.
Signed-off-by: shychee <shychee96@gmail.com>
