-
Notifications
You must be signed in to change notification settings - Fork 2
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
MARP-1294 Create a central monitoring reporting for security issues of axonivy marketplace #251
Conversation
this.isLoading = false; | ||
} | ||
|
||
formatCommitDate(date: string): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
duplicated: similar to time-ago pipe
|
||
private handleError(err: any): void { | ||
this.errorMessage = | ||
err.status === 401 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use const in common.constant.ts
isLoading = false; | ||
|
||
private securityMonitorService = inject(SecurityMonitorService); | ||
private readonly githubBaseUrl = 'https://github.com/axonivy-market'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
github -> gitHub
Should we define a constant?
@@ -0,0 +1,20 @@ | |||
export interface Repo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name of interface does not match to model file.
In BE site, you used ProductSecurityInfo. I think you can use this name for this model
status: string; | ||
alerts: Record<string, number>; | ||
}; | ||
secretsScanning: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
secretsScanning -> secretScanning
try { | ||
ResponseEntity<List<Map<String, Object>>> response = fetchApiResponseAsList(accessToken, | ||
String.format(GitHubConstants.Url.REPO_DEPENDABOT_ALERTS_OPEN, organization.getLogin(), repo.getName())); | ||
dependabot.setStatus(com.axonivy.market.enums.AccessLevel.ENABLED); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add import static for enum ENABLED instead of using full path of enum class in a method.
Update also for DISABLED and NO_PERMISSION.
} | ||
|
||
@NoArgsConstructor(access = AccessLevel.PRIVATE) | ||
public static class Url { | ||
private static final String BASE_URL = "https://api.github.com"; | ||
public static final String USER = BASE_URL + "/user"; | ||
public static final String REPO_SECURITY_ADVISORIES = BASE_URL + "/repos/%s/%s/security-advisories?state=%s"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this constant is not used.
return dependabot; | ||
} | ||
|
||
public static SecretScanning getNumberOfSecretScanningAlerts(GHRepository repo, GHOrganization organization, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Format code of this method
log.warn(e); | ||
dependabot.setStatus(com.axonivy.market.enums.AccessLevel.NO_PERMISSION); | ||
} | ||
return dependabot; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use finally block
return secretScanning; | ||
} | ||
|
||
public static CodeScanning getCodeScanningAlerts(GHRepository repo, GHOrganization organization, String accessToken) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similar to getDependabotAlerts(), could you unify them?
marketplace-ui/src/app/modules/product/product-detail/product-detail.service.spec.ts
Dismissed
Show dismissed
Hide dismissed
marketplace-ui/src/app/modules/product/product-detail/product-detail.service.spec.ts
Dismissed
Show dismissed
Hide dismissed
marketplace-ui/src/app/modules/security-monitor/security-monitor.service.spec.ts
Dismissed
Show dismissed
Hide dismissed
marketplace-ui/src/app/modules/security-monitor/security-monitor.service.spec.ts
Dismissed
Show dismissed
Hide dismissed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix sonarqube also
@@ -0,0 +1,227 @@ | |||
body { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should not change the styles of the body tag. It can affect the marketplace website.
@@ -18,7 +18,7 @@ import { TimeAgo } from '../enums/time-ago.enum'; | |||
}) | |||
export class TimeAgoPipe implements PipeTransform { | |||
translateService = inject(TranslateService); | |||
async transform(value?: Date, language?: Language, _args?: []): Promise<string> { | |||
async transform(value?: Date, language: Language = Language.EN, _args?: []): Promise<string> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check carefully with the time ago of the feature feedback time.
|
||
securityInfoList.sort(Comparator.comparing(ProductSecurityInfo::getRepoName)); | ||
} catch (IOException e) { | ||
throw new RuntimeException("Error fetching repository data", e); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should log it, and return an empty list when the system cannot fetch a repository
…ring-reporting-for-security-issues-of-axonivy-marketplace
import org.springframework.web.client.RestTemplate; | ||
|
||
@Configuration | ||
public class RestTemplateConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this class?
@@ -2,21 +2,39 @@ | |||
|
|||
import com.axonivy.market.bo.Artifact; | |||
import com.axonivy.market.constants.CommonConstants; | |||
import com.axonivy.market.constants.GitHubConstants; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove unused imports
if (err.status === UNAUTHORIZED ) { | ||
this.errorMessage = 'Unauthorized access.'; | ||
} | ||
else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format code
|
||
navigateToRepoPage(repoName: string, page: RepoPage, lastCommitSHA?: string): void { | ||
const paths: Record<RepoPage, string> = { | ||
security: '/security', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you define constants to avoid using hard-code?
const diffInSeconds = Math.floor((now - targetDate) / 1000); | ||
|
||
const timeUnits = [ | ||
{ seconds: 60, singular: 'minute', plural: 'minutes' }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you define an enum for timeUnits?
|
||
private loadSessionData(): void { | ||
const sessionData = sessionStorage.getItem(this.sessionKeys.data); | ||
if (sessionData) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you move lines 46-47 into try
block code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
No description provided.