Yicong-Huang commented on code in PR #7664: URL: https://github.com/apache/texera/pull/7664#discussion_r3919036023
########## LICENSE: ########## @@ -238,6 +238,15 @@ This product includes an icon from Google's Material Symbols: Source: https://github.com/google/material-design-icons License: Apache License 2.0 (this LICENSE file) +This product includes the ORCID iD icon from ORCID, Inc.: + - frontend/src/assets/logos/ORCID-iD_icon_24x24.png Review Comment: Not about this line — about the file it names. `ORCID-iD_icon_24x24.png` is committed with mode `100755`, while every other file in `frontend/src/assets/logos/` is `100644`. The executable bit is inert on a PNG, so this is tidiness only: ``` git update-index --chmod=-x frontend/src/assets/logos/ORCID-iD_icon_24x24.png ``` The LICENSE entry itself reads well — source, licence, bundled text and the trademark-vs-graphic note are all there. ########## frontend/src/app/hub/component/login/orcid-callback.component.ts: ########## @@ -0,0 +1,150 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { HttpErrorResponse } from "@angular/common/http"; +import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; +import { Component, OnInit } from "@angular/core"; +import { ActivatedRoute, Router } from "@angular/router"; +import { catchError, skip, take } from "rxjs/operators"; +import { EMPTY } from "rxjs"; +import { NzSpinComponent } from "ng-zorro-antd/spin"; +import { UserService } from "../../../common/service/user/user.service"; +import { NotificationService } from "../../../common/service/notification/notification.service"; +import { ORCID_STATE_KEY } from "../../../common/service/user/orcid-auth.service"; +import { LOGIN, USER_WORKFLOW } from "../../../app-routing.constant"; + +/** + * Where ORCID sends the browser back to after its consent screen, carrying the one-time `code` + * that only the backend can redeem (see `OrcidAuthResource`). Nothing here is interactive: it + * checks the round trip was one we started, hands the code over, and leaves. + */ +@UntilDestroy() +@Component({ + selector: "texera-orcid-callback", + template: ` + <div class="orcid-callback"> + <nz-spin nzSimple></nz-spin> + <p>Signing you in with ORCID…</p> + </div> + `, + styles: [ + ` + .orcid-callback { + display: flex; + flex-direction: column; + align-items: center; + justify-content: center; + gap: 16px; + height: 100vh; + } + `, + ], + imports: [NzSpinComponent], +}) +export class OrcidCallbackComponent implements OnInit { + constructor( + private route: ActivatedRoute, + private router: Router, + private userService: UserService, + private notificationService: NotificationService + ) {} + + ngOnInit(): void { + const params = this.route.snapshot.queryParamMap; + + const expectedState = sessionStorage.getItem(ORCID_STATE_KEY); + + //remove key to prevent leakage that would authorize future sessions Review Comment: Two small things. This is the one comment in the file that is not a formed sentence — no space after `//`, no capital, no period — while every sibling here is. More usefully, "authorize future sessions" overstates what the value does. `state` is a CSRF correlator, not a credential: a leftover one would let an unrelated callback *pass* the check below, never authorize anything. The spec above says it better ("good for exactly one round trip"). ```suggestion // Good for one round trip only: a leftover value would let an unrelated callback pass the check below. ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
